date:20130327

On Wed, Mar 27, 2013 at 12:48:07PM +0200, Mika Kuoppala wrote:
 Add write-verify test to gem_fence_thrash. Test will create
 multiple threads per fence then verify the write into fenced region.
 
 v2: non-threaded, non-tiled tests added. suggested by Chris Wilson.
 
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com

Slurped in, thanks for the patch. Imo Chris' idea to add support for cpu
mask can be best done as a patch on top of this. While looking at the test
I've noticed that you've added subtest support, but didn't move the test
to the multi-test target. Without that piglit integration breaks (since
only tests in that target are correclty enumerate for subtests).

Thanks, Daniel
 ---
  tests/gem_fence_thrash.c |  145 
 --
  1 file changed, 126 insertions(+), 19 deletions(-)
 
 diff --git a/tests/gem_fence_thrash.c b/tests/gem_fence_thrash.c
 index c9e847b..373c4d0 100644
 --- a/tests/gem_fence_thrash.c
 +++ b/tests/gem_fence_thrash.c
 @@ -55,15 +55,21 @@
   * when copying between two buffers and thus constantly swapping fences.
   */
  
 +struct test {
 + int fd;
 + int tiling;
 + int num_surfaces;
 +};
 +
  static void *
 -bo_create (int fd)
 +bo_create (int fd, int tiling)
  {
   void *ptr;
   int handle;
  
   handle = gem_create(fd, OBJECT_SIZE);
  
 - gem_set_tiling(fd, handle, I915_TILING_X, 1024);
 + gem_set_tiling(fd, handle, tiling, 1024);
  
   ptr = gem_mmap(fd, handle, OBJECT_SIZE, PROT_READ | PROT_WRITE);
  
 @@ -76,12 +82,13 @@ bo_create (int fd)
  static void *
  bo_copy (void *_arg)
  {
 - int fd = *(int *)_arg;
 + struct test *t = (struct test *)_arg;
 + int fd = t-fd;
   int n;
   char *a, *b;
  
 - a = bo_create (fd);
 - b = bo_create (fd);
 + a = bo_create (fd, t-tiling);
 + b = bo_create (fd, t-tiling);
  
   for (n = 0; n  1000; n++) {
   memcpy (a, b, OBJECT_SIZE);
 @@ -91,31 +98,131 @@ bo_copy (void *_arg)
   return NULL;
  }
  
 -int
 -main(int argc, char **argv)
 +static void
 +_bo_write_verify(struct test *t)
  {
 + int fd = t-fd;
 + int i, k;
 + volatile uint32_t **s;
 + uint32_t v;
 + unsigned int dwords = OBJECT_SIZE  2;
 + const char *tile_str[] = { none, x, y };
 +
 + assert (t-tiling = 0  t-tiling = I915_TILING_Y);
 + assert (t-num_surfaces  0);
 +
 + s = calloc(sizeof(**s), t-num_surfaces);
 +
 + for (k = 0; k  t-num_surfaces; k++) {
 + s[k] = bo_create(fd, t-tiling);
 + }
 +
 + for (k = 0; k  t-num_surfaces; k++) {
 + volatile uint32_t *a = s[k];
 +
 + for (i = 0; i  dwords; i++) {
 + a[i] = i;
 + v = a[i];
 + if (v != i) {
 + printf(tiling %s: write failed at %d (%x)\n,
 +tile_str[t-tiling], i, v);
 + _exit(-1);
 + }
 + }
 +
 + for (i = 0; i  dwords; i++) {
 + v = a[i];
 + if (v != i) {
 + printf(tiling %s: verify failed at %d (%x)\n,
 +tile_str[t-tiling], i, v);
 + exit(-2);
 + }
 + }
 + }
 +
 + free(s);
 +}
 +
 +static void *
 +bo_write_verify(void *_arg)
 +{
 + struct test *t = (struct test *)_arg;
 + int i;
 +
 + for (i = 0; i  10; i++)
 + _bo_write_verify(t);
 +
 + return 0;
 +}
 +
 +static int run_test(int threads_per_fence, void *f, int tiling,
 + int surfaces_per_thread)
 +{
 + struct test t;
   drm_i915_getparam_t gp;
 - pthread_t threads[32];
 - int n, num_fences;
 - int fd, ret;
 + pthread_t *threads;
 + int n, num_fences, num_threads;
 + int ret;
  
 - fd = drm_open_any();
 + t.fd = drm_open_any();
 + t.tiling = tiling;
 + t.num_surfaces = surfaces_per_thread;
  
   gp.param = I915_PARAM_NUM_FENCES_AVAIL;
   gp.value = num_fences;
 - ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
 + ret = ioctl(t.fd, DRM_IOCTL_I915_GETPARAM, gp);
   assert (ret == 0);
  
 - printf (creating %d threads\n, num_fences);
 - assert (num_fences  sizeof (threads) / sizeof (threads[0]));
 + num_threads = threads_per_fence * num_fences;
 +
 + printf(%s: threads %d, fences %d, tiling %d, surfaces per thread %d\n,
 +f == bo_copy ? copy : write-verify, num_threads,
 +num_fences, tiling, surfaces_per_thread);
 +
 + if (threads_per_fence) {
 + threads = calloc(sizeof(*threads), num_threads);
 + assert  (threads != NULL);
 +
 + for (n = 0; n  num_threads; n++)
 + pthread_create (threads[n], NULL, f, t);
 +
 + for (n = 0; n  num_threads; n++)
 + pthread_join

Re: [Intel-gfx] [PATCH 3/3] intel_perf_counters: Add support for Sandybridge.

On Tue, Mar 26, 2013 at 10:06:39PM -0700, Kenneth Graunke wrote:
 While the Sandybridge PRM doesn't have any documentation on the GPU's
 performance counters, a lot of information can be gleaned from the older
 Ironlake PRM.  Oddly, none of the information documented there actually
 appears to apply to Ironlake.  However, it apparently works just great
 on Sandybridge.
 
 Since this information has all been publicly available on the internet
 for around three years, we can use it.
 
 Signed-off-by: Kenneth Graunke kenn...@whitecape.org

Merged, thanks for the patches.
-Daniel

 ---
  tools/intel_perf_counters.c | 146 
 
  1 file changed, 146 insertions(+)
 
 diff --git a/tools/intel_perf_counters.c b/tools/intel_perf_counters.c
 index fd268b1..b528361 100644
 --- a/tools/intel_perf_counters.c
 +++ b/tools/intel_perf_counters.c
 @@ -22,9 +22,21 @@
   *
   * Authors:
   *Eric Anholt e...@anholt.net
 + *Kenneth Graunke kenn...@whitecape.org
 + *
 + * While documentation for performance counters is suspiciously missing from 
 the
 + * Sandybridge PRM, they were documented in Volume 1 Part 3 of the Ironlake 
 PRM.
 + *
 + * A lot of the Ironlake PRM actually unintentionally documents Sandybridge
 + * due to mistakes made when updating the documentation for Gen6+.  Many of
 + * these mislabeled sections carried forward to the public documentation.
 + *
 + * The Ironlake PRMs have been publicly available since 2010 and are online 
 at:
 + * 
 https://01.org/linuxgraphics/documentation/2010-intel-core-processor-family
   */
  
  #include unistd.h
 +#include stdbool.h
  #include stdlib.h
  #include stdio.h
  #include err.h
 @@ -71,6 +83,60 @@ const char *gen5_counter_names[GEN5_COUNTER_COUNT] = {
   cycles any EU is stalled for math,
  };
  
 +#define GEN6_COUNTER_COUNT 29
 +
 +/**
 + * Sandybridge: Counter Select = 001
 + * A0   A1   A2   A3   A4   TIMESTAMP RPT_ID
 + * A5   A6   A7   A8   A9   A10  A11  A12
 + * A13  A14  A15  A16  A17  A18  A19  A20
 + * A21  A22  A23  A24  A25  A26  A27  A28
 + */
 +const int gen6_counter_format = 1;
 +
 +/**
 + * Names for aggregating counters A0-A28.
 + *
 + * While the Ironlake PRM clearly documents that there are 29 counters 
 (A0-A28),
 + * it only lists the names for 28 of them; one is missing.  However, careful
 + * examination reveals a pattern: there are five GS counters (Active, Stall,
 + * Core Stall, # threads loaded, and ready but not running time).  There are
 + * also five PS counters, in the same order.  But there are only four VS
 + * counters listed - the number of VS threads loaded is missing.  Presumably,
 + * it exists and is counter 5, and the rest are shifted over one place.
 + */
 +const char *gen6_counter_names[GEN6_COUNTER_COUNT] = {
 + [0]  = Aggregated Core Array Active,
 + [1]  = Aggregated Core Array Stalled,
 + [2]  = Vertex Shader Active Time,
 + [3]  = Vertex Shader Stall Time,
 + [4]  = Vertex Shader Stall Time - Core Stall,
 + [5]  = # VS threads loaded,
 + [6]  = Vertex Shader Ready but not running time,
 + [7]  = Geometry Shader Active Time,
 + [8]  = Geometry Shader Stall Time,
 + [9]  = Geometry Shader Stall Time - Core Stall,
 + [10] = # GS threads loaded,
 + [11] = Geometry Shader ready but not running Time,
 + [12] = Pixel Shader Active Time,
 + [13] = Pixel Shader Stall Time,
 + [14] = Pixel Shader Stall Time - Core Stall,
 + [15] = # PS threads loaded,
 + [16] = Pixel Shader ready but not running Time,
 + [17] = Early Z Test Pixels Passing,
 + [18] = Early Z Test Pixels Failing,
 + [19] = Early Stencil Test Pixels Passing,
 + [20] = Early Stencil Test Pixels Failing,
 + [21] = Pixel Kill Count,
 + [22] = Alpha Test Pixels Failed,
 + [23] = Post PS Stencil Pixels Failed,
 + [24] = Post PS Z buffer Pixels Failed,
 + [25] = Pixels/samples Written in the frame buffer,
 + [26] = GPU Busy,
 + [27] = CL active and not stalled,
 + [28] = SF active and stalled,
 +};
 +
  int have_totals = 0;
  uint32_t *totals;
  uint32_t *last_counter;
 @@ -85,6 +151,20 @@ struct intel_batchbuffer *batch;
  #define MI_COUNTER_ADDRESS_GTT   (1  0)
  /* DW2: report ID */
  
 +/**
 + * According to the Sandybridge PRM, Volume 1, Part 1, page 48,
 + * MI_REPORT_PERF_COUNT is now opcode 0x28.  The Ironlake PRM, Volume 1,
 + * Part 3 details how it works.
 + */
 +/* DW0 */
 +#define GEN6_MI_REPORT_PERF_COUNT (0x28  23)
 +/* DW1 and 2 are the same as above */
 +
 +/* OACONTROL exists on Gen6+ but is documented in the Ironlake PRM */
 +#define OACONTROL   0x2360
 +# define OACONTROL_COUNTER_SELECT_SHIFT 2
 +# define PERFORMANCE_COUNTER_ENABLE (1  0)
 +
  static void
  gen5_get_counters(void)
  {
 @@ -124,6 +204,45 @@ gen5_get_counters(void)
   drm_intel_bo_unreference(stats_bo);
  }
  
 +static void
 +gen6_get_counters(void)
 +{
 + int i;
 + drm_intel_bo

Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine

2013-03-27 Thread Imre Deak

On Tue, 2013-03-26 at 16:07 -0700, Jesse Barnes wrote:
 On Wed, 20 Mar 2013 14:23:48 +0200
 Imre Deak imre.d...@intel.com wrote:
 
   + if (!info-screen_base)
  
  kfree(info-apertures) is missing. The same goes for
  intel_fbdev_destroy().
 
 Fixed in both places.
 
  
   + goto err_cmap;
   +
   + /* If the object is shmemfs backed, it will have given us zeroed pages.
   +  * If the object is stolen however, it will be full of whatever
   +  * garbage was left in there.
   +  */
   + if (ifbdev-ifb.obj-stolen)
   + memset_io(info-screen_base, 0, info-screen_size);
   +
   + /* Use default scratch pixmap (info-pixmap.flags = FB_PIXMAP_SYSTEM) */
   +
   + drm_fb_helper_fill_fix(info, fb-pitches[0], fb-depth);
   + drm_fb_helper_fill_var(info, ifbdev-helper, fb-width, fb-height);
   +
   + return info;
   +
   +err_cmap:
   + if (info-cmap.len)
   + fb_dealloc_cmap(info-cmap);
  
  Should be fine to call w/o checking cmap.len.
 
 Fixed in both places.
 
  
   +err_info:
   + framebuffer_release(info);
   + return NULL;
   +}
   +
static int intelfb_create(struct intel_fbdev *ifbdev,
   struct drm_fb_helper_surface_size *sizes)
{
 struct drm_device *dev = ifbdev-helper.dev;
   - struct drm_i915_private *dev_priv = dev-dev_private;
   - struct fb_info *info;
   - struct drm_framebuffer *fb;
   - struct drm_mode_fb_cmd2 mode_cmd = {};
   + struct drm_mode_fb_cmd2 mode_cmd = { 0 };
 struct drm_i915_gem_object *obj;
   - struct device *device = dev-pdev-dev;
   + struct fb_info *info;
 int size, ret;

 /* we don't do packed 24bpp */
 if (sizes-surface_bpp == 24)
 sizes-surface_bpp = 32;

   - mode_cmd.width = sizes-surface_width;
   + mode_cmd.width  = sizes-surface_width;
 mode_cmd.height = sizes-surface_height;

   - mode_cmd.pitches[0] = ALIGN(mode_cmd.width * ((sizes-surface_bpp + 7) /
   -   8), 64);
   - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes-surface_bpp,
   -   sizes-surface_depth);
   + mode_cmd.pitches[0] =
   + intel_framebuffer_pitch_for_width(mode_cmd.width,
   +   sizes-surface_bpp);
  
  This changes the way pitches[0] is calculated for surface_bpp % 8 != 0,
  but there's no mention of it in the commit message.
 
 It just removes the open coding; we still do the rounding and alignment
 to 64 bytes.

Yea, but you get different results due to the different way of rounding
for certain bpps. For example:

sizes-surface_bpp = 4
mode_cmd.width = 1000

You get pitches[0]=1024 with the old way and 512 with the new way.

--Imre


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.

2013-03-27 Thread Danny Baumann


Hi,


Well, the ACPI spec says this (section B.5.2):


The OEM may define the number 0 as Zero brightness that can mean
to turn off the lighting (e.g. LCD panel backlight) in the device.
This may be useful in the case of an output device that can still be
viewed using only ambient light, for example, a transflective LCD.


My interpretation of this is that the value 0 is supposed to still
be visible. I'm pretty sure I saw a statement that 0 is supposed to
mean barely visible somewhere, but can't find it at the moment.
I'll search for the source of it.


I think that's a stretch - This may be useful isn't normative
language, The OEM may define is. But even if we do assert it for the
ACPI backlight, it's not true for other interfaces - zero backlight
intensity is supposed to be screen off on Apple hardware, for instance.


OK, I see. And there is user space depending on that behaviour? And 
again - how is user space supposed to know about the behavioral 
differences? Is it something like 'if type is raw, don't expect anything'?
The reason for my question is that I want to determine what a) the 
correct place to fix this and b) the correct fix is. As Xrandr abstracts 
away the used backlight interface, I see no way for user space using 
Xrandr (e.g. KDE) to meaningfully handle this.


Thanks,

Danny
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 If for example the BIOS fb is too small for the dual pipe config we detect,
 we may have valid timings and such, but no fb. The pfit case also hits this
 path (though currently only fastboots if you hack your mode clock to match).

But even when the BIOS fb is to small and the BIOS config uses the
pfit, we /should/ have an fb around. Looking a bit closer I think the
confusion stems from you reading out the adjusted mode, but treating
it like the requested mode. I think it'd be much more solid if we
store the read-out mode in the adjusted_mode (won't work otherwise
with hw state cross-checking anyway), and then do two comparisons
here:

- Does the request mode (plus everything else) match? If so, just do
an fb_set_base call.
- Does the adjusted mode match (plus the entire output routing ofc)?
That means there's either the vga plane or a pfit in-between which we
don't like. If possible we can then play some tricks to avoid the full
modeset.

The reason that I'm a bit freaked about about your change here is that
in a lot of places we treat crtc-fb == NULL as no mode set. So I
fear we're breaking a few too many assumptions here with that hack,
and it simply needs more thought about what we should actually check.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.

2013-03-27 Thread Danny Baumann

Hi,

Well, the ACPI spec says this (section B.5.2):

The OEM may define the number 0 as Zero brightness that can mean
to turn off the lighting (e.g. LCD panel backlight) in the device.
This may be useful in the case of an output device that can still be
viewed using only ambient light, for example, a transflective LCD.

My interpretation of this is that the value 0 is supposed to still
be visible. I'm pretty sure I saw a statement that 0 is supposed to
mean barely visible somewhere, but can't find it at the moment.
I'll search for the source of it.

BTW, I found the source for that statement: [1], section
System.Client.BrightnessControls.SmoothBrightness. While formally it's
not part of the ACPI spec, I'm pretty sure most vendors (except Apple,
obviously) will follow it as if it were, if not more strictly.

OK, I see. And there is user space depending on that behaviour? And again -
how is user space supposed to know about the behavioral differences? Is it
something like 'if type is raw, don't expect anything'?
The reason for my question is that I want to determine what a) the correct
place to fix this and b) the correct fix is. As Xrandr abstracts away the
used backlight interface, I see no way for user space using Xrandr (e.g.
KDE) to meaningfully handle this.

In practice does it really matter? As a user if you set the
brightness really low and you either can't see the screen or can
barely make it out does it matter if the screen is off or just really,
really dim? The 0 brightness setting is probably not practically
usable regardless of what it does.

That's right. I'm not intending to use the laptop with that low
brightness, though, I'd just like to distinguish between my laptop being
turned off / suspended or just its display being dimmed down by the
desktop environment to conserve power. In order to do the latter, KDE
sets brightness to 0 ([2]), which worked fine for me as long as
acpi_video was working on this laptop. This isn't the case at present,
which is why I'm using intel_backlight at the moment. As you may have
noticed, things aren't working as expected with it, which in turn is
what brought me over here ;) I'm fine with sending a patch to KDE if
that's the correct thing to do, but I'm not yet sure what the correct
thing to do is.

Thanks,

Danny

[1] http://msdn.microsoft.com/en-us/library/windows/hardware/jj128256.aspx
[2]
https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/master/entry/powerdevil/daemon/actions/bundled/dimdisplay.cpp#L53

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.

2013-03-27 Thread Alex Deucher

On Wed, Mar 27, 2013 at 8:56 AM, Danny Baumann dannybaum...@web.de wrote:
 Hi,

 Well, the ACPI spec says this (section B.5.2):

 
 The OEM may define the number 0 as Zero brightness that can mean
 to turn off the lighting (e.g. LCD panel backlight) in the device.
 This may be useful in the case of an output device that can still be
 viewed using only ambient light, for example, a transflective LCD.
 

 My interpretation of this is that the value 0 is supposed to still
 be visible. I'm pretty sure I saw a statement that 0 is supposed to
 mean barely visible somewhere, but can't find it at the moment.
 I'll search for the source of it.


 BTW, I found the source for that statement: [1], section
 System.Client.BrightnessControls.SmoothBrightness. While formally it's not
 part of the ACPI spec, I'm pretty sure most vendors (except Apple,
 obviously) will follow it as if it were, if not more strictly.

 OK, I see. And there is user space depending on that behaviour? And again
 -
 how is user space supposed to know about the behavioral differences? Is
 it
 something like 'if type is raw, don't expect anything'?
 The reason for my question is that I want to determine what a) the
 correct
 place to fix this and b) the correct fix is. As Xrandr abstracts away the
 used backlight interface, I see no way for user space using Xrandr (e.g.
 KDE) to meaningfully handle this.


 In practice does it really matter?  As a user if you set the
 brightness really low and you either can't see the screen or can
 barely make it out does it matter if the screen is off or just really,
 really dim?  The 0 brightness setting is probably not practically
 usable regardless of what it does.


 That's right. I'm not intending to use the laptop with that low brightness,
 though, I'd just like to distinguish between my laptop being turned off /
 suspended or just its display being dimmed down by the desktop environment
 to conserve power. In order to do the latter, KDE sets brightness to 0
 ([2]), which worked fine for me as long as acpi_video was working on this
 laptop. This isn't the case at present, which is why I'm using
 intel_backlight at the moment. As you may have noticed, things aren't
 working as expected with it, which in turn is what brought me over here ;)
 I'm fine with sending a patch to KDE if that's the correct thing to do, but
 I'm not yet sure what the correct thing to do is.

FWIW, when I implemented native backlight support in the radeon
driver, I special cased level 0 as off since that was what a lot of
the other native backlight drivers did.  I'm open to changing it if
there is a plan for some kind of consistency, but it seems pretty
random at the moment.

Alex
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] regression: grass turns red

2013-03-27 Thread Hans de Bruin


On 03/26/2013 10:15 PM, Alexander van Heukelum wrote:

Hi Hans,

Could you check if the attached patch solves your problem?



Yep, the grass is green again.

--
Hans

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-27 Thread Chris Wilson

On Wed, Mar 27, 2013 at 01:49:47PM +0100, Daniel Vetter wrote:
 On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  If for example the BIOS fb is too small for the dual pipe config we detect,
  we may have valid timings and such, but no fb. The pfit case also hits this
  path (though currently only fastboots if you hack your mode clock to match).
 
 But even when the BIOS fb is to small and the BIOS config uses the
 pfit, we /should/ have an fb around. Looking a bit closer I think the
 confusion stems from you reading out the adjusted mode, but treating
 it like the requested mode. I think it'd be much more solid if we
 store the read-out mode in the adjusted_mode (won't work otherwise
 with hw state cross-checking anyway), and then do two comparisons
 here:
 
 - Does the request mode (plus everything else) match? If so, just do
 an fb_set_base call.
 - Does the adjusted mode match (plus the entire output routing ofc)?
 That means there's either the vga plane or a pfit in-between which we
 don't like. If possible we can then play some tricks to avoid the full
 modeset.
 
 The reason that I'm a bit freaked about about your change here is that
 in a lot of places we treat crtc-fb == NULL as no mode set. So I
 fear we're breaking a few too many assumptions here with that hack,
 and it simply needs more thought about what we should actually check.

The check here is why we were so enthusiastic about moving to a
pipe_config framework that could match up a configured pipe with an
invalid fb and then just exchange the fb.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: Split the framebuffer_info creation into a separate routine

2013-03-27 Thread Chris Wilson

On Wed, Mar 27, 2013 at 01:49:06PM +0200, Imre Deak wrote:
 On Tue, 2013-03-26 at 16:07 -0700, Jesse Barnes wrote:
  On Wed, 20 Mar 2013 14:23:48 +0200
  Imre Deak imre.d...@intel.com wrote:
  
+   if (!info-screen_base)
   
   kfree(info-apertures) is missing. The same goes for
   intel_fbdev_destroy().
  
  Fixed in both places.
  
   
+   goto err_cmap;
+
+   /* If the object is shmemfs backed, it will have given us 
zeroed pages.
+* If the object is stolen however, it will be full of whatever
+* garbage was left in there.
+*/
+   if (ifbdev-ifb.obj-stolen)
+   memset_io(info-screen_base, 0, info-screen_size);
+
+   /* Use default scratch pixmap (info-pixmap.flags = 
FB_PIXMAP_SYSTEM) */
+
+   drm_fb_helper_fill_fix(info, fb-pitches[0], fb-depth);
+   drm_fb_helper_fill_var(info, ifbdev-helper, fb-width, 
fb-height);
+
+   return info;
+
+err_cmap:
+   if (info-cmap.len)
+   fb_dealloc_cmap(info-cmap);
   
   Should be fine to call w/o checking cmap.len.
  
  Fixed in both places.
  
   
+err_info:
+   framebuffer_release(info);
+   return NULL;
+}
+
 static int intelfb_create(struct intel_fbdev *ifbdev,
  struct drm_fb_helper_surface_size *sizes)
 {
struct drm_device *dev = ifbdev-helper.dev;
-   struct drm_i915_private *dev_priv = dev-dev_private;
-   struct fb_info *info;
-   struct drm_framebuffer *fb;
-   struct drm_mode_fb_cmd2 mode_cmd = {};
+   struct drm_mode_fb_cmd2 mode_cmd = { 0 };
struct drm_i915_gem_object *obj;
-   struct device *device = dev-pdev-dev;
+   struct fb_info *info;
int size, ret;
 
/* we don't do packed 24bpp */
if (sizes-surface_bpp == 24)
sizes-surface_bpp = 32;
 
-   mode_cmd.width = sizes-surface_width;
+   mode_cmd.width  = sizes-surface_width;
mode_cmd.height = sizes-surface_height;
 
-   mode_cmd.pitches[0] = ALIGN(mode_cmd.width * 
((sizes-surface_bpp + 7) /
- 8), 64);
-   mode_cmd.pixel_format = 
drm_mode_legacy_fb_format(sizes-surface_bpp,
- 
sizes-surface_depth);
+   mode_cmd.pitches[0] =
+   intel_framebuffer_pitch_for_width(mode_cmd.width,
+ sizes-surface_bpp);
   
   This changes the way pitches[0] is calculated for surface_bpp % 8 != 0,
   but there's no mention of it in the commit message.
  
  It just removes the open coding; we still do the rounding and alignment
  to 64 bytes.
 
 Yea, but you get different results due to the different way of rounding
 for certain bpps. For example:
 
 sizes-surface_bpp = 4
 mode_cmd.width = 1000
 
 You get pitches[0]=1024 with the old way and 512 with the new way.

Not a bug in Jesse's patch though, but an earlier one of mine trying to
use the kernel macros and failing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.

2013-03-27 Thread Matthew Garrett

On Tue, Mar 26, 2013 at 12:48:44PM +0100, Danny Baumann wrote:
 This patch makes the behaviour of the intel_backlight backlight device
 consistent to e.g. acpi_videoX: When writing the value 0, the set brightness
 makes the panel content barely readable instead of turning the backlight off.
 This matches the expectations of user space (e.g. kde-workspace or the Intel
 X11 driver), which expects that it can use intel_backlight as a drop-in
 replacement for acpi_videoX.

I'm not quite clear what you mean here. The behaviour of 0 isn't well 
defined for the ACPI backlight driver - it's perfectly reasonable for it 
to turn the backlight off entirely. Anything assuming that 0 is still 
visible is broken.

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] regression: grass turns red

2013-03-27 Thread Alexander van Heukelum

Hi Hans,

Could you check if the attached patch solves your problem?

Greetings,
Alexander van Heukelum

On Sun, Mar 24, 2013, at 22:19, Hans de Bruin wrote:
 commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old' 
 somehow breaks the colors when I play 'civilization I' under xdosemu. 
 During the intro of the game something the colors get messed up. When 
 the game begins the grass of the earth is red. Reverting the  commit 
 fixes the problem.
 
 -- 
 Hans
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 
 
From 2b09f37fd9defc02c6da9900e23418a401d7a2b9 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum heuke...@fastmail.fm
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

This might solve the issue of the red grass in 'civilization I' under xdosemu. Commit
49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old' got rid of the pt_regs
stub for sys_vm86old and sys_vm86. The functions were, however, not changed to use the
asmlinkage calling convention.

Signed-off-by: Alexander van Heukelum heuke...@fastmail.fm
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c   | 6 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..a245b88 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage int sys_vm86old(struct vm86_struct __user *);
+asmlinkage int sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..7f72807 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+asmlinkage int sys_vm86old(struct vm86_struct __user *v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+asmlinkage int sys_vm86(unsigned long cmd, unsigned long arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Fix build failure

2013-03-27 Thread Lauri Kasanen

ERROR: __build_bug_on_failed [drivers/gpu/drm/i915/i915.ko] undefined!

Originally reported at 
http://www.gossamer-threads.com/lists/linux/kernel/1631803
FDO bug #62775

This needs to be backported to both 3.7 and 3.8 stable trees. Doesn't apply 
straight,
but it's a quick change.

Signed-off-by: Lauri Kasanen c...@gmx.com
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3b11ab0..9a48e1a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -57,7 +57,7 @@ eb_create(struct drm_i915_gem_execbuffer2 *args)
if (eb == NULL) {
int size = args-buffer_count;
int count = PAGE_SIZE / sizeof(struct hlist_head) / 2;
-   BUILD_BUG_ON(!is_power_of_2(PAGE_SIZE / sizeof(struct 
hlist_head)));
+   BUILD_BUG_ON_NOT_POWER_OF_2(PAGE_SIZE / sizeof(struct 
hlist_head));
while (count  2*size)
count = 1;
eb = kzalloc(count*sizeof(struct hlist_head) +
-- 
1.7.2.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup

2013-03-27 Thread Ville Syrjälä

On Wed, Mar 27, 2013 at 12:44:30AM +0100, Daniel Vetter wrote:
 Due to the irq setup rework in 3.9 Egbert's hpd rework blows up on
 pch-split platforms. The new init sequence is:
 
 - irq enabling
 - modeset init
 - hpd setup
 
 We need to move around the ibx setup a bit to fix this.
 
 This needs to be squashed into a commit on dinq.
 
 Cc: Egbert Eich e...@suse.de
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_irq.c | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 43436e0..1279a44 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -2084,7 +2084,7 @@ static void ibx_enable_hotplug(struct drm_device *dev)
   I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
  }
  
 -static void ibx_irq_postinstall(struct drm_device *dev)
 +static void ibx_hpd_irq_setup(struct drm_device *dev)
  {
   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
   struct drm_mode_config *mode_config = dev-mode_config;
 @@ -2095,12 +2095,10 @@ static void ibx_irq_postinstall(struct drm_device 
 *dev)
   mask = ~SDE_HOTPLUG_MASK;
   list_for_each_entry(intel_encoder, mode_config-encoder_list, 
 base.head)
   mask |= hpd_ibx[intel_encoder-hpd_pin];
 - mask |= SDE_GMBUS | SDE_AUX_MASK;
   } else {
   mask = ~SDE_HOTPLUG_MASK_CPT;
   list_for_each_entry(intel_encoder, mode_config-encoder_list, 
 base.head)
   mask |= hpd_cpt[intel_encoder-hpd_pin];
 - mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
   }
   I915_WRITE(SDEIIR, I915_READ(SDEIIR));


   I915_WRITE(SDEIMR, ~mask);
 @@ -2110,6 +2108,21 @@ static void ibx_irq_postinstall(struct drm_device *dev)
   ibx_enable_hotplug(dev);
  }
  
 +static void ibx_irq_postinstall(struct drm_device *dev)
 +{
 + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
 + u32 mask = I915_READ(SDEIER);
 +
 + if (HAS_PCH_IBX(dev))
 + mask |= SDE_GMBUS | SDE_AUX_MASK;
 + else
 + mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
 + I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 + I915_WRITE(SDEIMR, ~mask);
 + I915_WRITE(SDEIER, mask);
 + POSTING_READ(SDEIER);
 +}

Should we clear just the HPD bits here? Then again, I suppose
enable_hotplug_processing should make sure we don't do aux/gmbus
transfers at the same time, so maybe it doesn't matter.

But now I started to wondee what are chances that we'd get
some other interrupt while executing this function. That
could lead to a corrupted SDEIER now that the irq handler
touches it as well.

 +
  static int ironlake_irq_postinstall(struct drm_device *dev)
  {
   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
 @@ -2960,6 +2973,7 @@ void intel_irq_init(struct drm_device *dev)
   dev-driver-irq_uninstall = ironlake_irq_uninstall;
   dev-driver-enable_vblank = ivybridge_enable_vblank;
   dev-driver-disable_vblank = ivybridge_disable_vblank;
 + dev_priv-display.hpd_irq_setup = ibx_hpd_irq_setup;
   } else if (HAS_PCH_SPLIT(dev)) {
   dev-driver-irq_handler = ironlake_irq_handler;
   dev-driver-irq_preinstall = ironlake_irq_preinstall;
 @@ -2967,6 +2981,7 @@ void intel_irq_init(struct drm_device *dev)
   dev-driver-irq_uninstall = ironlake_irq_uninstall;
   dev-driver-enable_vblank = ironlake_enable_vblank;
   dev-driver-disable_vblank = ironlake_disable_vblank;
 + dev_priv-display.hpd_irq_setup = ibx_hpd_irq_setup;
   } else {
   if (INTEL_INFO(dev)-gen == 2) {
   dev-driver-irq_preinstall = i8xx_irq_preinstall;
 -- 
 1.7.11.7
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v2

2013-03-27 Thread Imre Deak

On Tue, 2013-03-26 at 23:53 +, Chris Wilson wrote:
 On Tue, Mar 26, 2013 at 04:33:06PM -0700, Jesse Barnes wrote:
  v2: check for non-native modes and adjust (Jesse)
  fixup aperture and cmap frees (Imre)
 The aperture is already freed via framebuffer_release(). And wrong
 patch?

Ah, haven't checked in there.. So aperture shouldn't be freed by us.

--Imre


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: clear crt hotplug compare voltage field before setting

Noticed while reviewing the hotplug irq setup code. Just looks better.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c697580..be2d6dd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2776,6 +2776,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
*/
if (IS_G4X(dev))
hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+   hotplug_en = ~CRT_HOTPLUG_VOLTAGE_COMPARE_MASK;
hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
 
/* Ignore TV since it's buggy */
-- 
1.7.11.7

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup

This fixes a regression introduced in

commit e5868a318d1ae28f760f77bb91ce5deb751733fd
Author: Egbert Eich e...@suse.de
Date:   Thu Feb 28 04:17:12 2013 -0500

DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encode

Due to the irq setup rework in 3.9, see

commit 20afbda209d708be66944907966486d0c1331cb8
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date:   Tue Dec 11 14:05:07 2012 +0100

drm/i915: Fixup hpd irq register setup ordering

Egbert Eich's hpd rework blows up on pch-split platforms - it walks
the encoder list before that has been set up completely. The new init
sequence is:

1. irq enabling
2. modeset init
3. hpd setup

We need to move around the ibx setup a bit to fix this.

Ville Syrjälä pointed out in his review that we can't touch SDEIER
after the interrupt handler is set up, since that'll race with Paulo
Zanoni's PCH interrupt race fix:

commit 44498aea293b37af1d463acd9658cdce1ecdf427
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date:   Fri Feb 22 17:05:28 2013 -0300

drm/i915: also disable south interrupts when handling them

We fix that by unconditionally enabling all interrupts in SDEIER, but
masking them as-needed in SDEIMR. Since only the single-threaded
setup/teardown (or suspend/resume) code touches that, no further
locking is required.

While at it also simplify the mask handling - we start out with all
interrupts cleared in the postinstall hook, and never enable a hpd
interrupt before hpd_irq_setup is called.

And finally, for consistency rename the ibx hpd setup function to
ibx_hpd_irq_setup.

v2: Fix race around SDEIER writes (Ville).

v3: Remove the superflous posting read for SDEIER, spotted by Ville.

Ville also wondered whether we shouldn't clear SDEIIR, since now
SDE interrupts are enabled before we have an irq handler installed.
But the master interrupt control bit in DEIER is still cleared, so we
should be fine.

Cc: Egbert Eich e...@suse.de
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_irq.c | 63 +++--
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 43436e0..666a0ec 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2028,7 +2028,13 @@ static void ironlake_irq_preinstall(struct drm_device 
*dev)
 
/* south display irq */
I915_WRITE(SDEIMR, 0x);
-   I915_WRITE(SDEIER, 0x0);
+   /*
+* SDEIER is also touched by the interrupt handler to work around missed
+* PCH interrupts. Hence we can't update it after the interrupt handler
+* is enabled - instead we unconditionally enable all PCH interrupt
+* sources here, but then only unmask them as needed with SDEIMR.
+*/
+   I915_WRITE(SDEIER, 0x);
POSTING_READ(SDEIER);
 }
 
@@ -2064,18 +2070,30 @@ static void valleyview_irq_preinstall(struct drm_device 
*dev)
POSTING_READ(VLV_IER);
 }
 
-/*
- * Enable digital hotplug on the PCH, and configure the DP short pulse
- * duration to 2ms (which is the minimum in the Display Port spec)
- *
- * This register is the same on all known PCH chips.
- */
-
-static void ibx_enable_hotplug(struct drm_device *dev)
+static void ibx_hpd_irq_setup(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
-   u32 hotplug;
+   struct drm_mode_config *mode_config = dev-mode_config;
+   struct intel_encoder *intel_encoder;
+   u32 mask = ~I915_READ(SDEIMR);
+   u32 hotplug;
+
+   if (HAS_PCH_IBX(dev)) {
+   list_for_each_entry(intel_encoder, mode_config-encoder_list, 
base.head)
+   mask |= hpd_ibx[intel_encoder-hpd_pin];
+   } else {
+   list_for_each_entry(intel_encoder, mode_config-encoder_list, 
base.head)
+   mask |= hpd_cpt[intel_encoder-hpd_pin];
+   }
 
+   I915_WRITE(SDEIMR, ~mask);
+
+   /*
+* Enable digital hotplug on the PCH, and configure the DP short pulse
+* duration to 2ms (which is the minimum in the Display Port spec)
+*
+* This register is the same on all known PCH chips.
+*/
hotplug = I915_READ(PCH_PORT_HOTPLUG);
hotplug = 
~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
@@ -2087,27 +2105,14 @@ static void ibx_enable_hotplug(struct drm_device *dev)
 static void ibx_irq_postinstall(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
-   struct drm_mode_config *mode_config = dev-mode_config;
-   struct intel_encoder *intel_encoder;
-   u32 mask = I915_READ(SDEIER);

Re: [Intel-gfx] [PATCH] drm/i915: clear crt hotplug compare voltage field before setting

2013-03-27 Thread Ville Syrjälä

On Wed, Mar 27, 2013 at 03:47:11PM +0100, Daniel Vetter wrote:
 Noticed while reviewing the hotplug irq setup code. Just looks better.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

What about the activation period? Do we want to leave it alone on non
g4x platforms, or should it also get cleared?

 ---
  drivers/gpu/drm/i915/i915_irq.c | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index c697580..be2d6dd 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -2776,6 +2776,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
   */
   if (IS_G4X(dev))
   hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
 + hotplug_en = ~CRT_HOTPLUG_VOLTAGE_COMPARE_MASK;
   hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
  
   /* Ignore TV since it's buggy */
 -- 
 1.7.11.7
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup

2013-03-27 Thread Ville Syrjälä

On Wed, Mar 27, 2013 at 03:55:01PM +0100, Daniel Vetter wrote:
 This fixes a regression introduced in
 
 commit e5868a318d1ae28f760f77bb91ce5deb751733fd
 Author: Egbert Eich e...@suse.de
 Date:   Thu Feb 28 04:17:12 2013 -0500
 
 DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in 
 encode
 
 Due to the irq setup rework in 3.9, see
 
 commit 20afbda209d708be66944907966486d0c1331cb8
 Author: Daniel Vetter daniel.vet...@ffwll.ch
 Date:   Tue Dec 11 14:05:07 2012 +0100
 
 drm/i915: Fixup hpd irq register setup ordering
 
 Egbert Eich's hpd rework blows up on pch-split platforms - it walks
 the encoder list before that has been set up completely. The new init
 sequence is:
 
 1. irq enabling
 2. modeset init
 3. hpd setup
 
 We need to move around the ibx setup a bit to fix this.
 
 Ville Syrjälä pointed out in his review that we can't touch SDEIER
 after the interrupt handler is set up, since that'll race with Paulo
 Zanoni's PCH interrupt race fix:
 
 commit 44498aea293b37af1d463acd9658cdce1ecdf427
 Author: Paulo Zanoni paulo.r.zan...@intel.com
 Date:   Fri Feb 22 17:05:28 2013 -0300
 
 drm/i915: also disable south interrupts when handling them
 
 We fix that by unconditionally enabling all interrupts in SDEIER, but
 masking them as-needed in SDEIMR. Since only the single-threaded
 setup/teardown (or suspend/resume) code touches that, no further
 locking is required.
 
 While at it also simplify the mask handling - we start out with all
 interrupts cleared in the postinstall hook, and never enable a hpd
 interrupt before hpd_irq_setup is called.
 
 And finally, for consistency rename the ibx hpd setup function to
 ibx_hpd_irq_setup.
 
 v2: Fix race around SDEIER writes (Ville).
 
 v3: Remove the superflous posting read for SDEIER, spotted by Ville.
 
 Ville also wondered whether we shouldn't clear SDEIIR, since now
 SDE interrupts are enabled before we have an irq handler installed.
 But the master interrupt control bit in DEIER is still cleared, so we
 should be fine.
 
 Cc: Egbert Eich e...@suse.de
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Makes sense to me.

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

snip

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.

2013-03-27 Thread Egbert Eich

Daniel Vetter writes:
  On Mon, Feb 25, 2013 at 12:06:52PM -0500, Egbert Eich wrote:
   Now since we have replaced the bits to show interest in hotplug IRQs
   we can go and nuke the 'hotplug_supported_mask'.
   
   Signed-off-by: Egbert Eich e...@suse.de
  
  I've applied your patch up to this one. Patch four needed some manual
  convincing to fit correctly, but the new hpd infrastructure is now in
  place.
  
  To get the actual hpd irq storm detection going I think it'd be good to
  resend the remaining patches on top of latest drm-intel-nightly. I'll try
  to yell at people a bit harder to review things more timely this time
  around.
  
  Thanks for the patches.

Thanks guys for reviewing the patches. I will try to spare some time tonight
to look into the issues that came up. 
I got dragged into other tasks as well so I didn't even find the time to 
follow up on the patches myself.

For now I need to go back and debug yet another issue with SDVO on GM45 :(

Cheers,
Egbert.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on.

On Mon, Feb 25, 2013 at 12:06:57PM -0500, Egbert Eich wrote:
 We disable hoptplug detection when we encounter a hotplug event
 storm. Still hotplug detection is required on some outputs (like
 Display Port). The interrupt storm may be only temporary (on certain
 Dell Laptops for instance it happens at certain charging states of
 the system). Thus we enable it after a certain grace period (2 minutes).
 Should the interrupt storm persist it will be detected immediately
 and it will be disabled again.
 
 Signed-off-by: Egbert Eich e...@suse.de

Oops, just noticed that you have an ibx_hpd_irq_setup patch, too ;-) I
think I'll go with since that one's just gotten a bit a beating in review
from Ville. Hopfully that's not too ugly to rebase over ...
-Daniel

 ---
  drivers/gpu/drm/i915/i915_drv.h |2 +
  drivers/gpu/drm/i915/i915_irq.c |   53 
 ++-
  2 files changed, 54 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 6ca742d..58bee7a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1068,6 +1068,8 @@ typedef struct drm_i915_private {
   HPD_MARK_DISABLED = 2
   } hpd_mark;
   } hpd_stats[HPD_NUM_PINS];
 +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
 + struct timer_list hotplug_reenable_timer;
  } drm_i915_private_t;
  
  /* Iterate over initialised rings */
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 8878fdd..ba598e3 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -376,8 +376,11 @@ static void i915_hotplug_work_func(struct work_struct 
 *work)
   connector_disabled = true;
   }
   }
 - if (connector_disabled)
 + if (connector_disabled) {
   drm_kms_helper_poll_enable(dev); /* if there were no outputs to 
 poll, poll is disabled */
 + mod_timer(dev_priv-hotplug_reenable_timer,
 +   jiffies + 
 msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
 + }
  
   spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
  
 @@ -2253,6 +2256,8 @@ static void valleyview_irq_uninstall(struct drm_device 
 *dev)
   if (!dev_priv)
   return;
  
 + del_timer_sync(dev_priv-hotplug_reenable_timer);
 +
   for_each_pipe(pipe)
   I915_WRITE(PIPESTAT(pipe), 0x);
  
 @@ -2274,6 +2279,8 @@ static void ironlake_irq_uninstall(struct drm_device 
 *dev)
   if (!dev_priv)
   return;
  
 + del_timer_sync(dev_priv-hotplug_reenable_timer);
 +
   I915_WRITE(HWSTAM, 0x);
  
   I915_WRITE(DEIMR, 0x);
 @@ -2612,6 +2619,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
   int pipe;
  
 + del_timer_sync(dev_priv-hotplug_reenable_timer);
 +
   if (I915_HAS_HOTPLUG(dev)) {
   I915_WRITE(PORT_HOTPLUG_EN, 0);
   I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 @@ -2860,6 +2869,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
   if (!dev_priv)
   return;
  
 + del_timer_sync(dev_priv-hotplug_reenable_timer);
 +
   I915_WRITE(PORT_HOTPLUG_EN, 0);
   I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
  
 @@ -2875,6 +2886,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
   I915_WRITE(IIR, I915_READ(IIR));
  }
  
 +static void i915_reenable_hotplug_timer_func(unsigned long data)
 +{
 + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
 + struct drm_device *dev = dev_priv-dev;
 + struct drm_mode_config *mode_config = dev-mode_config;
 + unsigned long irqflags;
 + int i;
 +
 + spin_lock_irqsave(dev_priv-irq_lock, irqflags);
 + for (i = 1; i  HPD_NUM_PINS; i++) {
 + if (dev_priv-hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) {
 + struct drm_connector *connector;
 +
 + dev_priv-hpd_stats[i].hpd_mark = HPD_ENABLED;
 + list_for_each_entry(connector, 
 mode_config-connector_list, head) {
 + struct intel_connector *intel_connector = 
 to_intel_connector(connector);
 +
 + if (intel_connector-encoder-hpd_pin == i) {
 + if (connector-polled != 
 intel_connector-polled)
 + DRM_DEBUG_DRIVER(Reenabling 
 HPD on connector %s\n,
 +  
 drm_get_connector_name(connector));
 + connector-polled = 
 intel_connector-polled;
 + if (!connector-polled)
 + connector-polled = 
 DRM_CONNECTOR_POLL_HPD;
 + }
 +

Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.

2013-03-27 Thread Matthew Garrett

On Wed, Mar 27, 2013 at 12:56:37PM +0100, Danny Baumann wrote:

 OK, I see. And there is user space depending on that behaviour? And
 again - how is user space supposed to know about the behavioral
 differences? Is it something like 'if type is raw, don't expect
 anything'?

Do not rely upon 0 being visible.

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: clear crt hotplug compare voltage field before setting

On Wed, Mar 27, 2013 at 05:02:43PM +0200, Ville Syrjälä wrote:
 On Wed, Mar 27, 2013 at 03:47:11PM +0100, Daniel Vetter wrote:
  Noticed while reviewing the hotplug irq setup code. Just looks better.
  
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

Queued for -next, thanks for the review.

 What about the activation period? Do we want to leave it alone on non
 g4x platforms, or should it also get cleared?

The activation period is just one bit, so setting it will dtrt. And there
are other values to tune the crt hotplug detection, so I've figured I'll
leave things as-is otherwise. The lack of proper masking just annoyed my
OCD a bit ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: implement ibx_hpd_irq_setup

On Wed, Mar 27, 2013 at 05:05:54PM +0200, Ville Syrjälä wrote:
 On Wed, Mar 27, 2013 at 03:55:01PM +0100, Daniel Vetter wrote:
  This fixes a regression introduced in
  
  commit e5868a318d1ae28f760f77bb91ce5deb751733fd
  Author: Egbert Eich e...@suse.de
  Date:   Thu Feb 28 04:17:12 2013 -0500
  
  DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in 
  encode
  
  Due to the irq setup rework in 3.9, see
  
  commit 20afbda209d708be66944907966486d0c1331cb8
  Author: Daniel Vetter daniel.vet...@ffwll.ch
  Date:   Tue Dec 11 14:05:07 2012 +0100
  
  drm/i915: Fixup hpd irq register setup ordering
  
  Egbert Eich's hpd rework blows up on pch-split platforms - it walks
  the encoder list before that has been set up completely. The new init
  sequence is:
  
  1. irq enabling
  2. modeset init
  3. hpd setup
  
  We need to move around the ibx setup a bit to fix this.
  
  Ville Syrjälä pointed out in his review that we can't touch SDEIER
  after the interrupt handler is set up, since that'll race with Paulo
  Zanoni's PCH interrupt race fix:
  
  commit 44498aea293b37af1d463acd9658cdce1ecdf427
  Author: Paulo Zanoni paulo.r.zan...@intel.com
  Date:   Fri Feb 22 17:05:28 2013 -0300
  
  drm/i915: also disable south interrupts when handling them
  
  We fix that by unconditionally enabling all interrupts in SDEIER, but
  masking them as-needed in SDEIMR. Since only the single-threaded
  setup/teardown (or suspend/resume) code touches that, no further
  locking is required.
  
  While at it also simplify the mask handling - we start out with all
  interrupts cleared in the postinstall hook, and never enable a hpd
  interrupt before hpd_irq_setup is called.
  
  And finally, for consistency rename the ibx hpd setup function to
  ibx_hpd_irq_setup.
  
  v2: Fix race around SDEIER writes (Ville).
  
  v3: Remove the superflous posting read for SDEIER, spotted by Ville.
  
  Ville also wondered whether we shouldn't clear SDEIIR, since now
  SDE interrupts are enabled before we have an irq handler installed.
  But the master interrupt control bit in DEIER is still cleared, so we
  should be fine.
  
  Cc: Egbert Eich e...@suse.de
  Cc: Jesse Barnes jbar...@virtuousgeek.org
  Cc: Paulo Zanoni paulo.r.zan...@intel.com
  Cc: Ville Syrjälä ville.syrj...@linux.intel.com
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 
 Makes sense to me.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Queued for -next, thanks for the patch. I've just pushed down the patch a
bit to keep the bisect regression window small, but not squashed - keeping
a record of our discussion seemed beneficial.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 1/4] drm: Add struct drm_rect and assorted utility functions

From: Ville Syrjälä ville.syrj...@linux.intel.com

struct drm_rect represents a simple rectangle. The utility
functions are there to help driver writers.

v2: Moved the region stuff into its own file, made the smaller funcs
static inline, used 64bit maths in the scaled clipping function to
avoid overflows (instead it will saturate to INT_MIN or INT_MAX).
v3: Renamed drm_region to drm_rect, drm_region_clip to
drm_rect_intersect, and drm_region_subsample to drm_rect_downscale.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/Makefile   |   3 +-
 drivers/gpu/drm/drm_rect.c |  96 +
 include/drm/drm_rect.h | 132 +
 3 files changed, 230 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_rect.c
 create mode 100644 include/drm/drm_rect.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0d59b24..8f94018 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,8 @@ drm-y   :=drm_auth.o drm_buffer.o drm_bufs.o 
drm_cache.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_modes.o drm_edid.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
-   drm_trace_points.o drm_global.o drm_prime.o
+   drm_trace_points.o drm_global.o drm_prime.o \
+   drm_rect.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
new file mode 100644
index 000..1ad4f5e
--- /dev/null
+++ b/drivers/gpu/drm/drm_rect.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2011-2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
+ * SOFTWARE.
+ */
+
+#include linux/errno.h
+#include linux/export.h
+#include linux/kernel.h
+#include drm/drm_rect.h
+
+/**
+ * drm_rect_intersect - intersect two rectangles
+ * @r1: first rectangle
+ * @r2: second rectangle
+ *
+ * Calculate the intersection of rectangles @r1 and @r2.
+ * @r1 will be overwritten with the intersection.
+ *
+ * RETURNS:
+ * @true if rectangle @r1 is still visible after the operation,
+ * @false otherwise.
+ */
+bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
+{
+   r1-x1 = max(r1-x1, r2-x1);
+   r1-y1 = max(r1-y1, r2-y1);
+   r1-x2 = min(r1-x2, r2-x2);
+   r1-y2 = min(r1-y2, r2-y2);
+
+   return drm_rect_visible(r1);
+}
+EXPORT_SYMBOL(drm_rect_intersect);
+
+/**
+ * drm_rect_clip_scaled - perform a scaled clip operation
+ * @src: source window rectangle
+ * @dst: destination window rectangle
+ * @clip: clip rectangle
+ * @hscale: horizontal scaling factor
+ * @vscale: vertical scaling factor
+ *
+ * Clip rectangle @dst by rectangle @clip. Clip rectangle @src by the
+ * same amounts multiplied by @hscale and @vscale.
+ *
+ * RETUTRNS:
+ * @true if rectangle @dst is still visible after being clipped,
+ * @false otherwise
+ */
+bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
+ const struct drm_rect *clip,
+ int hscale, int vscale)
+{
+   int diff;
+
+   diff = clip-x1 - dst-x1;
+   if (diff  0) {
+   int64_t tmp = src-x1 + (int64_t) diff * hscale;
+   src-x1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
+   }
+   diff = clip-y1 - dst-y1;
+   if (diff  0) {
+   int64_t tmp = src-y1 + (int64_t) diff * vscale;
+   src-y1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
+   }
+   diff = dst-x2 - clip-x2;
+   if (diff  0) {
+   int64_t tmp = src-x2 - (int64_t) diff * hscale;
+   src-x2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
+   }
+   diff = dst-y2 - clip-y2;
+   if (diff  0) {
+   int64_t tmp =

[Intel-gfx] [PATCH v3 2/4] drm: Add drm_rect_calc_{hscale, vscale}() utility functions

From: Ville Syrjälä ville.syrj...@linux.intel.com

These functions calculcate the scaling factor based on the source and
destination rectangles.

There are two version of the functions, the strict ones that will
return an error if the min/max scaling factor is exceeded, and the
relaxed versions that will adjust the src/dst rectangles in order to
keep the scaling factor withing the limits.

v2: Return error instead of adjusting regions, refactor common parts
into one function, and split into strict and relaxed versions.
v3: Renamed drm_region to drm_rect, add _rect_ to the function
names.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/drm_rect.c | 177 +
 include/drm/drm_rect.h |  12 +++
 2 files changed, 189 insertions(+)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 1ad4f5e..8270ab4 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -94,3 +94,180 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct 
drm_rect *dst,
return drm_rect_intersect(dst, clip);
 }
 EXPORT_SYMBOL(drm_rect_clip_scaled);
+
+static int drm_calc_scale(int src, int dst)
+{
+   int scale = 0;
+
+   if (src  0 || dst  0)
+   return -EINVAL;
+
+   if (dst == 0)
+   return 0;
+
+   scale = src / dst;
+
+   return scale;
+}
+
+/**
+ * drm_rect_calc_hscale - calculate the horizontal scaling factor
+ * @src: source window rectangle
+ * @dst: destination window rectangle
+ * @min_hscale: minimum allowed horizontal scaling factor
+ * @max_hscale: maximum allowed horizontal scaling factor
+ *
+ * Calculate the horizontal scaling factor as
+ * (@src width) / (@dst width).
+ *
+ * RETURNS:
+ * The horizontal scaling factor, or errno of out of limits.
+ */
+int drm_rect_calc_hscale(const struct drm_rect *src,
+const struct drm_rect *dst,
+int min_hscale, int max_hscale)
+{
+   int src_w = drm_rect_width(src);
+   int dst_w = drm_rect_width(dst);
+   int hscale = drm_calc_scale(src_w, dst_w);
+
+   if (hscale  0 || dst_w == 0)
+   return hscale;
+
+   if (hscale  min_hscale || hscale  max_hscale)
+   return -ERANGE;
+
+   return hscale;
+}
+EXPORT_SYMBOL(drm_rect_calc_hscale);
+
+/**
+ * drm_rect_calc_vscale - calculate the vertical scaling factor
+ * @src: source window rectangle
+ * @dst: destination window rectangle
+ * @min_vscale: minimum allowed vertical scaling factor
+ * @max_vscale: maximum allowed vertical scaling factor
+ *
+ * Calculate the vertical scaling factor as
+ * (@src height) / (@dst height).
+ *
+ * RETURNS:
+ * The vertical scaling factor, or errno of out of limits.
+ */
+int drm_rect_calc_vscale(const struct drm_rect *src,
+const struct drm_rect *dst,
+int min_vscale, int max_vscale)
+{
+   int src_h = drm_rect_height(src);
+   int dst_h = drm_rect_height(dst);
+   int vscale = drm_calc_scale(src_h, dst_h);
+
+   if (vscale  0 || dst_h == 0)
+   return vscale;
+
+   if (vscale  min_vscale || vscale  max_vscale)
+   return -ERANGE;
+
+   return vscale;
+}
+EXPORT_SYMBOL(drm_rect_calc_vscale);
+
+/**
+ * drm_calc_hscale_relaxed - calculate the horizontal scaling factor
+ * @src: source window rectangle
+ * @dst: destination window rectangle
+ * @min_hscale: minimum allowed horizontal scaling factor
+ * @max_hscale: maximum allowed horizontal scaling factor
+ *
+ * Calculate the horizontal scaling factor as
+ * (@src width) / (@dst width).
+ *
+ * If the calculated scaling factor is below @min_vscale,
+ * decrease the height of rectangle @dst to compensate.
+ *
+ * If the calculcated scaling factor is above @max_vscale,
+ * decrease the height of rectangle @src to compensate.
+ *
+ * RETURNS:
+ * The horizontal scaling factor.
+ */
+int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
+struct drm_rect *dst,
+int min_hscale, int max_hscale)
+{
+   int src_w = drm_rect_width(src);
+   int dst_w = drm_rect_width(dst);
+   int hscale = drm_calc_scale(src_w, dst_w);
+
+   if (hscale  0 || dst_w == 0)
+   return hscale;
+
+   if (hscale  min_hscale) {
+   int max_dst_w = src_w / min_hscale;
+
+   drm_rect_adjust_size(dst, max_dst_w - dst_w, 0);
+
+   return min_hscale;
+   }
+
+   if (hscale  max_hscale) {
+   int max_src_w = dst_w * max_hscale;
+
+   drm_rect_adjust_size(src, max_src_w - src_w, 0);
+
+   return max_hscale;
+   }
+
+   return hscale;
+}
+EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
+
+/**
+ * drm_rect_calc_vscale_relaxed - calculate the vertical scaling factor
+ * @src: source window rectangle
+ * @dst: destination window rectangle
+ * @min_vscale:

[Intel-gfx] [PATCH v2 3/4] drm: Add drm_rect_debug_print()

From: Ville Syrjälä ville.syrj...@linux.intel.com

Add a debug function to print the rectangle in a human readable format.

v2: Renamed drm_region to drm_rect, the function from drm_region_debug
to drm_rect_debug_print(), and use %+d instead of +%d in the format.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/drm_rect.c | 22 ++
 include/drm/drm_rect.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 8270ab4..ed19b77 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -24,6 +24,7 @@
 #include linux/errno.h
 #include linux/export.h
 #include linux/kernel.h
+#include drm/drmP.h
 #include drm/drm_rect.h
 
 /**
@@ -271,3 +272,24 @@ int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
return vscale;
 }
 EXPORT_SYMBOL(drm_rect_calc_vscale_relaxed);
+
+/**
+ * drm_rect_debug_print - print the rectangle information
+ * @r: rectangle to print
+ * @fixed_point: rectangle is in 16.16 fixed point format
+ */
+void drm_rect_debug_print(const struct drm_rect *r, bool fixed_point)
+{
+   int w = drm_rect_width(r);
+   int h = drm_rect_height(r);
+
+   if (fixed_point)
+   DRM_DEBUG_KMS(%d.%06ux%d.%06u%+d.%06u%+d.%06u\n,
+ w  16, ((w  0x) * 15625)  10,
+ h  16, ((h  0x) * 15625)  10,
+ r-x1  16, ((r-x1  0x) * 15625)  10,
+ r-y1  16, ((r-y1  0x) * 15625)  10);
+   else
+   DRM_DEBUG_KMS(%ux%u%+d%+d\n, w, h, r-x1, r-y1);
+}
+EXPORT_SYMBOL(drm_rect_debug_print);
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 164e6ce..d5d4e1c 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -140,5 +140,6 @@ int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
 int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
 struct drm_rect *dst,
 int min_vscale, int max_vscale);
+void drm_rect_debug_print(const struct drm_rect *r, bool fixed_point);
 
 #endif
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 4/4] drm/i915: Implement proper clipping for video sprites

From: Ville Syrjälä ville.syrj...@linux.intel.com

Properly clip the source when the destination gets clipped
by the pipe dimensions.

Sadly the video sprite hardware is rather limited so it can't do proper
sub-pixel postitioning. Resort to truncating the source coordinates to
(macro)pixel boundary.

The scaling checks are done using the relaxed drm_region functions.
That means that the src/dst regions are reduced in size in order to keep
the scaling factor within the limits.

Also do some additional checking against various hardware limits.

v2: Truncate src coords instead of rounding to avoid increasing src
viewport size, and adapt to changes in drm_calc_{h,v}scale().
v3: Adapt to drm_region-drm_rect rename. Fix misaligned crtc_w for
packed YUV formats when scaling isn't supported.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_sprite.c | 191 +++-
 1 file changed, 145 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 414d325..62e09d1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -32,6 +32,7 @@
 #include drm/drmP.h
 #include drm/drm_crtc.h
 #include drm/drm_fourcc.h
+#include drm/drm_rect.h
 #include intel_drv.h
 #include drm/i915_drm.h
 #include i915_drv.h
@@ -415,6 +416,20 @@ ilk_get_colorkey(struct drm_plane *plane, struct 
drm_intel_sprite_colorkey *key)
key-flags = I915_SET_COLORKEY_NONE;
 }
 
+static bool
+format_is_yuv(uint32_t format)
+{
+   switch (format) {
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_YVYU:
+   return true;
+   default:
+   return false;
+   }
+}
+
 static int
 intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
@@ -432,9 +447,28 @@ intel_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
  pipe);
int ret = 0;
-   int x = src_x  16, y = src_y  16;
int primary_w = crtc-mode.hdisplay, primary_h = crtc-mode.vdisplay;
bool disable_primary = false;
+   bool visible;
+   int hscale, vscale;
+   int max_scale, min_scale;
+   int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
+   struct drm_rect src = {
+   .x1 = src_x,
+   .x2 = src_x + src_w,
+   .y1 = src_y,
+   .y2 = src_y + src_h,
+   };
+   struct drm_rect dst = {
+   .x1 = crtc_x,
+   .x2 = crtc_x + crtc_w,
+   .y1 = crtc_y,
+   .y2 = crtc_y + crtc_h,
+   };
+   const struct drm_rect clip = {
+   .x2 = crtc-mode.hdisplay,
+   .y2 = crtc-mode.vdisplay,
+   };
 
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
@@ -450,19 +484,23 @@ intel_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,
intel_plane-src_w = src_w;
intel_plane-src_h = src_h;
 
-   src_w = src_w  16;
-   src_h = src_h  16;
-
/* Pipe must be running... */
-   if (!(I915_READ(PIPECONF(cpu_transcoder))  PIPECONF_ENABLE))
-   return -EINVAL;
-
-   if (crtc_x = primary_w || crtc_y = primary_h)
+   if (!(I915_READ(PIPECONF(cpu_transcoder))  PIPECONF_ENABLE)) {
+   DRM_DEBUG_KMS(Pipe disabled\n);
return -EINVAL;
+   }
 
/* Don't modify another pipe's plane */
-   if (intel_plane-pipe != intel_crtc-pipe)
+   if (intel_plane-pipe != intel_crtc-pipe) {
+   DRM_DEBUG_KMS(Wrong plane - crtc mapping\n);
return -EINVAL;
+   }
+
+   /* FIXME check all gen limits */
+   if (fb-width  3 || fb-height  3 || fb-pitches[0]  16384) {
+   DRM_DEBUG_KMS(Unsuitable framebuffer for plane\n);
+   return -EINVAL;
+   }
 
/* Sprite planes can be linear or x-tiled surfaces */
switch (obj-tiling_mode) {
@@ -470,53 +508,111 @@ intel_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,
case I915_TILING_X:
break;
default:
+   DRM_DEBUG_KMS(Unsupported tiling mode\n);
return -EINVAL;
}
 
/*
-* Clamp the width  height into the visible area.  Note we don't
-* try to scale the source if part of the visible region is offscreen.
-* The caller must handle that by adjusting source offset and size.
+* FIXME the following code does a bunch of fuzzy adjustments to the
+* coordinates and sizes. We probably need some way to decide whether
+* more strict checking

[Intel-gfx] [PATCH] drm/i915: Wait for vblank between disabling a sprite and unpinning the fb

From: Ville Syrjälä ville.syrj...@linux.intel.com

When disabling a sprite, wait for the sprite to stop fetching data
from memory before unpinning the fb.

Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 414d325..27df5b8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -582,6 +582,8 @@ intel_disable_plane(struct drm_plane *plane)
if (!intel_plane-obj)
goto out;
 
+   intel_wait_for_vblank(dev, intel_plane-pipe);
+
mutex_lock(dev-struct_mutex);
intel_unpin_fb_obj(intel_plane-obj);
intel_plane-obj = NULL;
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

On Wed, 27 Mar 2013 13:49:47 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  If for example the BIOS fb is too small for the dual pipe config we detect,
  we may have valid timings and such, but no fb. The pfit case also hits this
  path (though currently only fastboots if you hack your mode clock to match).
 
 But even when the BIOS fb is to small and the BIOS config uses the
 pfit, we /should/ have an fb around. Looking a bit closer I think the
 confusion stems from you reading out the adjusted mode, but treating
 it like the requested mode. I think it'd be much more solid if we
 store the read-out mode in the adjusted_mode (won't work otherwise
 with hw state cross-checking anyway), and then do two comparisons
 here:

Yeah, there's an fb, but it may not be what we want.  So we don't
allocate it and thus crtc-fb is empty.

 - Does the request mode (plus everything else) match? If so, just do
 an fb_set_base call.
 - Does the adjusted mode match (plus the entire output routing ofc)?
 That means there's either the vga plane or a pfit in-between which we
 don't like. If possible we can then play some tricks to avoid the full
 modeset.

Right, I'm trying to play tricks here for sure.

 The reason that I'm a bit freaked about about your change here is that
 in a lot of places we treat crtc-fb == NULL as no mode set. So I
 fear we're breaking a few too many assumptions here with that hack,
 and it simply needs more thought about what we should actually check.

Yeah I was worried about that too.  Maybe we should use crtc-active
for that everywhere?

With the modeset rework and pipe config stuff, we should be able to
drop this in favor of something that looks at the pipe_config and
figures out the minimal mode set sequence required.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Retrieve the current mode upon KMS takeover v2

On Wed, 27 Mar 2013 01:13:48 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Tue, Mar 26, 2013 at 04:33:07PM -0700, Jesse Barnes wrote:
  Read the current hardware state to retrieve the active mode and populate
  our CRTC config if that mode matches our presumptions.
  
  v2: check that get_hw_state gave us a valid pipe (Imre)
  add clock_get for ILK+ (Jesse)
  
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 
 Please preheat your wrath-dispenser ...
 
 Atm the mode retrieval logic is smashed into setup_hw_state. Imo this
 needs to be part of the general hw state readout, and for paranoia needs
 to be of the usual cross-checking after each modeset.

I was thinking about this last night too; I don't like reading the
state in the fb layer either, it really belongs in intel_display
somewhere.

 Some later patches from my pipe_config series (after the pieces just
 resend) add some basic infrastructure for this, including lax matching
 ruels (e.g. for the clock cross-checking after a modeset, since we don't
 yet put the _real_ hw dotclock into adjusted_mode-clock).

I'll check it out.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Wait for vblank between disabling a sprite and unpinning the fb

On Wed, 27 Mar 2013 17:49:13 +0200
ville.syrj...@linux.intel.com wrote:

 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 When disabling a sprite, wait for the sprite to stop fetching data
 from memory before unpinning the fb.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_sprite.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 b/drivers/gpu/drm/i915/intel_sprite.c
 index 414d325..27df5b8 100644
 --- a/drivers/gpu/drm/i915/intel_sprite.c
 +++ b/drivers/gpu/drm/i915/intel_sprite.c
 @@ -582,6 +582,8 @@ intel_disable_plane(struct drm_plane *plane)
   if (!intel_plane-obj)
   goto out;
  
 + intel_wait_for_vblank(dev, intel_plane-pipe);
 +
   mutex_lock(dev-struct_mutex);
   intel_unpin_fb_obj(intel_plane-obj);
   intel_plane-obj = NULL;

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/13] drm/i915: introduce struct intel_crtc_config

On Wed, 27 Mar 2013 00:44:50 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 Currently only containing the requested and the adjusted mode. And
 only crtc callbacks are converted somewhat to it, encoders will be
 done on a as-needed basis (simply too much churn in one patch
 otherwise).
 
 Future patches will add tons more useful stuff to this struct,
 starting with the very simple.
 
 v2: Store the pipe_config in the intel_crtc, so that the -mode-set,
 -enable and also -disable have easy access to it.
 
 v3: Store the pipe config in the right crtc ...
 
 v4: Rebased.
 
 v5: Fixup an OOPS when trying to kfree an ERR_PTR.
 
 v6: Used drm_moode_copy and some other small cleanups as suggested
 by Ville Syrjälä.
 
 v7: drm_mode_copy preserves the mode id of the destination, so no need
 to clear it again (Ville).
 
 v8: Break a long line spotted by Paulo.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---

Yeah looks like this could be applied immediately.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/13] drm/i915: compute pipe_config earlier

On Wed, 27 Mar 2013 00:44:51 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 To make decent modeset state checking possible (e.g. for the check
 mode with atomic modesetting) we want to have the full pipe
 configuration and state checks done before we touch the hw.
 
 To ensure that all the little bitspieces that are now moved to the
 pipe_config handle this correctly, move its computation to the right
 spot now, before we touch the hw in the disable_pipes step.
 
 Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 34986fe..56ff8a5 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7829,12 +7829,6 @@ int intel_set_mode(struct drm_crtc *crtc,
   intel_modeset_affected_pipes(crtc, modeset_pipes,
prepare_pipes, disable_pipes);
  
 - DRM_DEBUG_KMS(set mode pipe masks: modeset: %x, prepare: %x, disable: 
 %x\n,
 -   modeset_pipes, prepare_pipes, disable_pipes);
 -
 - for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 - intel_crtc_disable(intel_crtc-base);
 -
   *saved_hwmode = crtc-hwmode;
   *saved_mode = crtc-mode;
  
 @@ -7853,6 +7847,12 @@ int intel_set_mode(struct drm_crtc *crtc,
   }
   }
  
 + DRM_DEBUG_KMS(set mode pipe masks: modeset: %x, prepare: %x, disable: 
 %x\n,
 +   modeset_pipes, prepare_pipes, disable_pipes);
 +
 + for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 + intel_crtc_disable(intel_crtc-base);
 +
   for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
   if (intel_crtc-base.enabled)
   dev_priv-display.crtc_disable(intel_crtc-base);

Looks safe :)

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Wait for vblank between disabling a sprite and unpinning the fb

On Wed, Mar 27, 2013 at 09:29:03AM -0700, Jesse Barnes wrote:
 On Wed, 27 Mar 2013 17:49:13 +0200
 ville.syrj...@linux.intel.com wrote:
 
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  When disabling a sprite, wait for the sprite to stop fetching data
  from memory before unpinning the fb.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/intel_sprite.c | 2 ++
   1 file changed, 2 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
  b/drivers/gpu/drm/i915/intel_sprite.c
  index 414d325..27df5b8 100644
  --- a/drivers/gpu/drm/i915/intel_sprite.c
  +++ b/drivers/gpu/drm/i915/intel_sprite.c
  @@ -582,6 +582,8 @@ intel_disable_plane(struct drm_plane *plane)
  if (!intel_plane-obj)
  goto out;
   
  +   intel_wait_for_vblank(dev, intel_plane-pipe);
  +
  mutex_lock(dev-struct_mutex);
  intel_unpin_fb_obj(intel_plane-obj);
  intel_plane-obj = NULL;
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, 27 Mar 2013 00:44:52 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 Only used by the lvds encoder. Note that we shouldn't do the same
 simple conversion with the FORCE_6BPC flag, since that's much better
 handled by moving all the pipe_bpc computation around.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 12 +++-
  drivers/gpu/drm/i915/intel_drv.h | 10 ++
  drivers/gpu/drm/i915/intel_lvds.c| 19 +--
  3 files changed, 26 insertions(+), 15 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 56ff8a5..3e22305 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3970,7 +3970,7 @@ static bool intel_crtc_compute_config(struct drm_crtc 
 *crtc,
   /* All interlaced capable intel hw wants timings in frames. Note though
* that intel_lvds_mode_fixup does some funny tricks with the crtc
* timings, so we need to be careful not to clobber these.*/
 - if (!(adjusted_mode-private_flags  INTEL_MODE_CRTC_TIMINGS_SET))
 + if (!pipe_config-timings_set)
   drm_mode_set_crtcinfo(adjusted_mode, 0);
  
   /* WaPruneModeWithIncorrectHsyncOffset: Cantiga+ cannot handle modes
 @@ -7532,6 +7532,16 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
  
   if (encoder-new_crtc-base != crtc)
   continue;
 +
 + if (encoder-compute_config) {
 + if (!(encoder-compute_config(encoder, pipe_config))) {
 + DRM_DEBUG_KMS(Encoder config failure\n);
 + goto fail;
 + }
 +
 + continue;
 + }
 +
   encoder_funcs = encoder-base.helper_private;
   if (!(encoder_funcs-mode_fixup(encoder-base,
   pipe_config-requested_mode,
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 4cc6625..054032a 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -105,10 +105,6 @@
  #define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0)
  #define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf  
 INTEL_MODE_PIXEL_MULTIPLIER_SHIFT)
  #define INTEL_MODE_DP_FORCE_6BPC (0x10)
 -/* This flag must be set by the encoder's mode_fixup if it changes the crtc
 - * timings in the mode to prevent the crtc fixup from overwriting them.
 - * Currently only lvds needs that. */
 -#define INTEL_MODE_CRTC_TIMINGS_SET (0x20)
  /*
   * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
   * to be used.
 @@ -158,6 +154,8 @@ struct intel_encoder {
   bool cloneable;
   bool connectors_active;
   void (*hot_plug)(struct intel_encoder *);
 + bool (*compute_config)(struct intel_encoder *,
 +struct intel_crtc_config *);
   void (*pre_pll_enable)(struct intel_encoder *);
   void (*pre_enable)(struct intel_encoder *);
   void (*enable)(struct intel_encoder *);
 @@ -203,6 +201,10 @@ struct intel_connector {
  struct intel_crtc_config {
   struct drm_display_mode requested_mode;
   struct drm_display_mode adjusted_mode;
 + /* This flag must be set by the encoder's compute_config callback if it
 +  * changes the crtc timings in the mode to prevent the crtc fixup from
 +  * overwriting them.  Currently only lvds needs that. */
 + bool timings_set;
  };
  
  struct intel_crtc {
 diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
 b/drivers/gpu/drm/i915/intel_lvds.c
 index 6ff145f..a2c516c 100644
 --- a/drivers/gpu/drm/i915/intel_lvds.c
 +++ b/drivers/gpu/drm/i915/intel_lvds.c
 @@ -261,8 +261,6 @@ centre_horizontally(struct drm_display_mode *mode,
  
   mode-crtc_hsync_start = mode-crtc_hblank_start + sync_pos;
   mode-crtc_hsync_end = mode-crtc_hsync_start + sync_width;
 -
 - mode-private_flags |= INTEL_MODE_CRTC_TIMINGS_SET;
  }
  
  static void
 @@ -284,8 +282,6 @@ centre_vertically(struct drm_display_mode *mode,
  
   mode-crtc_vsync_start = mode-crtc_vblank_start + sync_pos;
   mode-crtc_vsync_end = mode-crtc_vsync_start + sync_width;
 -
 - mode-private_flags |= INTEL_MODE_CRTC_TIMINGS_SET;
  }
  
  static inline u32 panel_fitter_scaling(u32 source, u32 target)
 @@ -301,15 +297,17 @@ static inline u32 panel_fitter_scaling(u32 source, u32 
 target)
   return (FACTOR * ratio + FACTOR/2) / FACTOR;
  }
  
 -static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 -   const struct drm_display_mode *mode,
 -   struct drm_display_mode *adjusted_mode)
 +static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 +   struct intel_crtc_config *pipe_config)
  {
 - struct drm_device *dev = encoder-dev;
 + struct drm_device *dev

Re: [Intel-gfx] [PATCH 04/13] drm/i915: add pipe_config-pixel_multiplier

On Wed, 27 Mar 2013 00:44:53 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 Used by SDVO (and hopefully, eventually HDMI, if we ever get around
 to fixing up the low dotclock CEA modes ...).
 
 This required adding a new encoder-mode_set callback to be able to
 pass around the intel_crtc_config.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 80 
 +++-
  drivers/gpu/drm/i915/intel_drv.h | 19 ++---
  drivers/gpu/drm/i915/intel_sdvo.c| 39 +-
  3 files changed, 66 insertions(+), 72 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 3e22305..3672b8d 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4320,14 +4320,15 @@ static void i9xx_update_pll_dividers(struct drm_crtc 
 *crtc,
  }
  
  static void vlv_update_pll(struct drm_crtc *crtc,
 -struct drm_display_mode *mode,
 -struct drm_display_mode *adjusted_mode,
  intel_clock_t *clock, intel_clock_t *reduced_clock,
  int num_connectors)
  {
   struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 + struct drm_display_mode *adjusted_mode =
 + intel_crtc-config.adjusted_mode;
 + struct drm_display_mode *mode = intel_crtc-config.requested_mode;

These arg compaction changes could probably be squashed into the
initial crtc_config patch to make this one smaller.

 @@ -5907,8 +5909,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
   encoder-base.base.id,
   drm_get_encoder_name(encoder-base),
   mode-base.id, mode-name);
 - encoder_funcs = encoder-base.helper_private;
 - encoder_funcs-mode_set(encoder-base, mode, adjusted_mode);
 + if (encoder-mode_set) {
 + encoder-mode_set(encoder);
 + } else {
 + encoder_funcs = encoder-base.helper_private;
 + encoder_funcs-mode_set(encoder-base, mode, 
 adjusted_mode);
 + }
   }

This made me do a double take; maybe it's time to
s/encoder/intel_encoder in this function...

Looks good otherwise.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/13] drm/i915: drop helper vtable for sdvo encoder

On Wed, 27 Mar 2013 00:44:54 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 Completely unused by now. Separate patch in case I've missed a
 place somewhere which dereferences the helper vtable but actually
 shouldn't do so.
 
 v2: Resolve rebase conflict with Egbert Eich's hpd infrastructure
 rework.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_sdvo.c | 5 -
  1 file changed, 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
 b/drivers/gpu/drm/i915/intel_sdvo.c
 index 4d9fede..6912742 100644
 --- a/drivers/gpu/drm/i915/intel_sdvo.c
 +++ b/drivers/gpu/drm/i915/intel_sdvo.c
 @@ -2041,9 +2041,6 @@ done:
  #undef CHECK_PROPERTY
  }
  
 -static const struct drm_encoder_helper_funcs intel_sdvo_helper_funcs = {
 -};
 -
  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
   .dpms = intel_sdvo_dpms,
   .detect = intel_sdvo_detect,
 @@ -2784,8 +2781,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t 
 sdvo_reg, bool is_sdvob)
   if (intel_sdvo-hotplug_active)
   intel_encoder-hpd_pin = HPD_SDVO_B ? HPD_SDVO_B : HPD_SDVO_C;
  
 - drm_encoder_helper_add(intel_encoder-base, intel_sdvo_helper_funcs);
 -
   intel_encoder-compute_config = intel_sdvo_compute_config;
   intel_encoder-disable = intel_disable_sdvo;
   intel_encoder-mode_set = intel_sdvo_mode_set;

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, Mar 27, 2013 at 09:49:52AM -0700, Jesse Barnes wrote:
 Changelog and summary could be better and mention the new
 compute_config function and how it replaces the mode_fixup one.
 
 Also, TIMINGS_SET probably wasn't a very good name in the first place,
 since it really deals with letter/pillar box configs.  But that's not
 really a problem with your patch and could be changed in a follow-on.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

Well, I've missed to add a commit addition discussed with Ville:


 
 -- 
 Jesse Barnes, Intel Open Source Technology Center
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, 27 Mar 2013 00:44:52 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 + bool (*compute_config)(struct intel_encoder *,
 +struct intel_crtc_config *);
   void (*pre_pll_enable)(struct intel_encoder *);
   void (*pre_enable)(struct intel_encoder *);
   void (*enable)(struct intel_encoder *);
 @@ -203,6 +201,10 @@ struct intel_connector {
  struct intel_crtc_config {
   struct drm_display_mode requested_mode;
   struct drm_display_mode adjusted_mode;
 + /* This flag must be set by the encoder's compute_config callback if it
 +  * changes the crtc timings in the mode to prevent the crtc fixup from
 +  * overwriting them.  Currently only lvds needs that. */
 + bool timings_set;

The compute_config function could actually use some kdoc instead of
putting it over the timings_set function.  It'll need to be expanded to
cover all the pipe_config bits eventually, what they mean and when they
should be set.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, Mar 27, 2013 at 5:49 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 Changelog and summary could be better and mention the new
 compute_config function and how it replaces the mode_fixup one.

 Also, TIMINGS_SET probably wasn't a very good name in the first place,
 since it really deals with letter/pillar box configs.  But that's not
 really a problem with your patch and could be changed in a follow-on.

 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

I've forgotten to add a commit addition discussed with Ville:

Note that since the lvds code unconditionally sets the crtc timings, we
can also unconditionally set the respective flag and not just when we set
special timings like the old code did.

I'll smash another paragraph for you on top (which also address an
issue raised by Paulo):

This requires that we pass the pipe config around to encoders, so
that they can set special attributes and set constraints. To do so
introduce a new -compute_config encoder callback, which is called in
stead of the drm crtc helper's -mode_fixup.

To avoid massive churn all over the codebase we don't want to convert
all existing -mode_fixup functions. Instead I've opted to convert
them on an as-needed basis (mostly to cut down on rebase conflicts and
to have more freedom to experiment around while developing the
patches).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/13] drm/i915: add pipe_config-pixel_multiplier

On Wed, Mar 27, 2013 at 5:54 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 3e22305..3672b8d 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4320,14 +4320,15 @@ static void i9xx_update_pll_dividers(struct drm_crtc 
 *crtc,
  }

  static void vlv_update_pll(struct drm_crtc *crtc,
 -struct drm_display_mode *mode,
 -struct drm_display_mode *adjusted_mode,
  intel_clock_t *clock, intel_clock_t *reduced_clock,
  int num_connectors)
  {
   struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 + struct drm_display_mode *adjusted_mode =
 + intel_crtc-config.adjusted_mode;
 + struct drm_display_mode *mode = intel_crtc-config.requested_mode;

 These arg compaction changes could probably be squashed into the
 initial crtc_config patch to make this one smaller.

See my little commit message update, but I've done things in such an
incremental way to avoid rebase hell and allow me to move things
around easier. What you see here is by far not my very first approach
;-)

To make everyone's OCD happy we can do a cleanup pass at the end, but
atm I'd like to avoid massive sed patches - too high churn.

 @@ -5907,8 +5909,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
   encoder-base.base.id,
   drm_get_encoder_name(encoder-base),
   mode-base.id, mode-name);
 - encoder_funcs = encoder-base.helper_private;
 - encoder_funcs-mode_set(encoder-base, mode, adjusted_mode);
 + if (encoder-mode_set) {
 + encoder-mode_set(encoder);
 + } else {
 + encoder_funcs = encoder-base.helper_private;
 + encoder_funcs-mode_set(encoder-base, mode, 
 adjusted_mode);
 + }
   }

 This made me do a double take; maybe it's time to
 s/encoder/intel_encoder in this function...

Yeah, we're halfway between mostly using intel_encoder and not so much
drm_encoder. Again, I think we can do a sed jobs once things settle,
meanwhile it's gonna be a bit ugly. Personally I don't care ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/13] drm/i915: add pipe_config-has_pch_encoder

On Wed, 27 Mar 2013 00:44:55 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 This is used way too often in the enable/disable paths. And will
 be even more useful in the future.
 
 Note that correct semantics of this change highly depend upon
 correct updating of intel_crtc-config: Like with all other
 modeset state, we need to call -disable with the old config,
 but -mode_set and -enable with the new config.
 
 v2: Do not yet use the flag in the -disable callbacks - atm we don't
 yet have support for the information stored in the pipe_config in the
 hw state readout code, so this will be wrong at boot-up/resume.
 
 v3: Rebased on top of the hdmi/dp ddi encoder merging.
 
 v4: Fixup stupid rebase error which lead to a NULL vfunc deref.
 
 v5: On haswell the VGA port is on the PCH!
 
 v6: s/IS_HASWELL/HAS_DDI/, spotted by Paulo Zanoni. Also add a missing
 parameter name in a function declaration.
 
 v7: Don't forget to git add ...

Looks like you got all the outputs covered.  But we always seem to get
this bit wrong, so we'll need to test on all the configs we know about
at least...

+   if (HAS_PCH_SPLIT(dev)  !HAS_DDI(dev)  !is_cpu_edp(intel_dp))
+   pipe_config-has_pch_encoder = true;
+

This could just do
 if (intel_dp-is_pch_edp)
pipe_config-has_pch_encoder = true;
right?  Since we cover the other cases in dp_init_connector?

But either way:

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, Mar 27, 2013 at 5:59 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 @@ -203,6 +201,10 @@ struct intel_connector {
  struct intel_crtc_config {
   struct drm_display_mode requested_mode;
   struct drm_display_mode adjusted_mode;
 + /* This flag must be set by the encoder's compute_config callback if it
 +  * changes the crtc timings in the mode to prevent the crtc fixup from
 +  * overwriting them.  Currently only lvds needs that. */
 + bool timings_set;

 The compute_config function could actually use some kdoc instead of
 putting it over the timings_set function.  It'll need to be expanded to
 cover all the pipe_config bits eventually, what they mean and when they
 should be set.

Now I very much like to claim the opposite, but this isn't designed
but very much organically grown code. So imo documentation doesn't
make too much sense before things settle a bit more (the auto fdi link
dither at the end will introduce quite a bit of fun still ...).

I've promised though in my pipe_config intro a few weeks ago that I'll
create a nice blog post and doc patch once the basic stuff is settled.
I still intend to deliver on that. Is that good enough?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/13] drm/i915: add pipe_config-limited_color_range

On Wed, 27 Mar 2013 00:44:56 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 Now that we have a useful struct for this, let's use it. Some neat
 pointer-chasing required, but it's all there already.
 
 v2: Rebased on top of the added Haswell limited color range support.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 13 ++---
  drivers/gpu/drm/i915/intel_dp.c  |  2 +-
  drivers/gpu/drm/i915/intel_drv.h | 12 +++-
  drivers/gpu/drm/i915/intel_hdmi.c|  5 +++--
  drivers/gpu/drm/i915/intel_sdvo.c|  5 +++--
  5 files changed, 20 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index fda0754..bfed546 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -5173,7 +5173,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
   else
   val |= PIPECONF_PROGRESSIVE;
  
 - if (adjusted_mode-private_flags  INTEL_MODE_LIMITED_COLOR_RANGE)
 + if (intel_crtc-config.limited_color_range)
   val |= PIPECONF_COLOR_RANGE_SELECT;
   else
   val = ~PIPECONF_COLOR_RANGE_SELECT;
 @@ -5189,8 +5189,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
   * is supported, but eventually this should handle various
   * RGB-YCbCr scenarios as well.
   */
 -static void intel_set_pipe_csc(struct drm_crtc *crtc,
 -const struct drm_display_mode *adjusted_mode)
 +static void intel_set_pipe_csc(struct drm_crtc *crtc)
  {
   struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
 @@ -5205,7 +5204,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc,
* consideration.
*/
  
 - if (adjusted_mode-private_flags  INTEL_MODE_LIMITED_COLOR_RANGE)
 + if (intel_crtc-config.limited_color_range)
   coeff = ((235 - 16) * (1  12) / 255)  0xff8; /* 0.xxx... */
  
   /*
 @@ -5229,7 +5228,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc,
   if (INTEL_INFO(dev)-gen  6) {
   uint16_t postoff = 0;
  
 - if (adjusted_mode-private_flags  
 INTEL_MODE_LIMITED_COLOR_RANGE)
 + if (intel_crtc-config.limited_color_range)
   postoff = (16 * (1  13) / 255)  0x1fff;
  
   I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
 @@ -5240,7 +5239,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc,
   } else {
   uint32_t mode = CSC_MODE_YUV_TO_RGB;
  
 - if (adjusted_mode-private_flags  
 INTEL_MODE_LIMITED_COLOR_RANGE)
 + if (intel_crtc-config.limited_color_range)
   mode |= CSC_BLACK_SCREEN_OFFSET;
  
   I915_WRITE(PIPE_CSC_MODE(pipe), mode);
 @@ -5841,7 +5840,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
  
   haswell_set_pipeconf(crtc, adjusted_mode, dither);
  
 - intel_set_pipe_csc(crtc, adjusted_mode);
 + intel_set_pipe_csc(crtc);
  
   /* Set up the display plane register */
   I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE | 
 DISPPLANE_PIPE_CSC_ENABLE);
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index bc73e5e..d7c1403 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -739,7 +739,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
   }
  
   if (intel_dp-color_range)
 - adjusted_mode-private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
 + pipe_config-limited_color_range = true;
  
   mode_rate = intel_dp_link_required(adjusted_mode-clock, bpp);
  
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 8de1855..63160c6 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -103,11 +103,6 @@
  
  /* drm_display_mode-private_flags */
  #define INTEL_MODE_DP_FORCE_6BPC (0x10)
 -/*
 - * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
 - * to be used.
 - */
 -#define INTEL_MODE_LIMITED_COLOR_RANGE (0x40)
  
  struct intel_framebuffer {
   struct drm_framebuffer base;
 @@ -193,6 +188,13 @@ struct intel_crtc_config {
   /* Whether to set up the PCH/FDI. Note that we never allow sharing
* between pch encoders and cpu encoders. */
   bool has_pch_encoder;
 +
 + /*
 +  * Use reduced/limited/broadcast rbg range, compressing from the full
 +  * range fed into the crtcs.
 +  */
 + bool limited_color_range;
 +
   /* Used by SDVO (and if we ever fix it, HDMI). */
   unsigned pixel_multiplier;
  };
 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index b588e6c..5508687 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -333,6 +333,7 @@ static void

Re: [Intel-gfx] [PATCH 08/13] drm/i915: introduce pipe_config-dither|pipe_bpp

On Wed, 27 Mar 2013 00:44:57 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 We want to compute this earlier. To avoid a big complicated patch,
 this patch here just does the big searchreplace and still calls the
 old functions at the same places.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_ddi.c |  8 
  drivers/gpu/drm/i915/intel_display.c | 25 +
  drivers/gpu/drm/i915/intel_dp.c  |  2 +-
  drivers/gpu/drm/i915/intel_drv.h |  4 +++-
  drivers/gpu/drm/i915/intel_hdmi.c|  2 +-
  5 files changed, 26 insertions(+), 15 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index baeb470..3d09df0 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -931,7 +931,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
   if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
  
   temp = TRANS_MSA_SYNC_CLK;
 - switch (intel_crtc-bpp) {
 + switch (intel_crtc-config.pipe_bpp) {
   case 18:
   temp |= TRANS_MSA_6_BPC;
   break;
 @@ -947,7 +947,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
   default:
   temp |= TRANS_MSA_8_BPC;
   WARN(1, %d bpp unsupported by DDI function\n,
 -  intel_crtc-bpp);
 +  intel_crtc-config.pipe_bpp);
   }
   I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
   }
 @@ -969,7 +969,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc 
 *crtc)
   temp = TRANS_DDI_FUNC_ENABLE;
   temp |= TRANS_DDI_SELECT_PORT(port);
  
 - switch (intel_crtc-bpp) {
 + switch (intel_crtc-config.pipe_bpp) {
   case 18:
   temp |= TRANS_DDI_BPC_6;
   break;
 @@ -984,7 +984,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc 
 *crtc)
   break;
   default:
   WARN(1, %d bpp unsupported by transcoder DDI function\n,
 -  intel_crtc-bpp);
 +  intel_crtc-config.pipe_bpp);
   }
  
   if (crtc-mode.flags  DRM_MODE_FLAG_PVSYNC)
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index bfed546..b495629 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4648,6 +4648,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
   const intel_limit_t *limit;
   int ret;
  
 + /* temporary hack */
 + intel_crtc-config.dither =
 + adjusted_mode-private_flags  INTEL_MODE_DP_FORCE_6BPC;
 +
   for_each_encoder_on_crtc(dev, crtc, encoder) {
   switch (encoder-type) {
   case INTEL_OUTPUT_LVDS:
 @@ -4748,7 +4752,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
   /* default to 8bpc */
   pipeconf = ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
   if (is_dp) {
 - if (adjusted_mode-private_flags  INTEL_MODE_DP_FORCE_6BPC) {
 + if (intel_crtc-config.dither) {
   pipeconf |= PIPECONF_6BPC |
   PIPECONF_DITHER_EN |
   PIPECONF_DITHER_TYPE_SP;
 @@ -4756,7 +4760,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
   }
  
   if (IS_VALLEYVIEW(dev)  intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
 - if (adjusted_mode-private_flags  INTEL_MODE_DP_FORCE_6BPC) {
 + if (intel_crtc-config.dither) {
   pipeconf |= PIPECONF_6BPC |
   PIPECONF_ENABLE |
   I965_PIPECONF_ACTIVE;
 @@ -5145,7 +5149,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
   val = I915_READ(PIPECONF(pipe));
  
   val = ~PIPECONF_BPC_MASK;
 - switch (intel_crtc-bpp) {
 + switch (intel_crtc-config.pipe_bpp) {
   case 18:
   val |= PIPECONF_6BPC;
   break;
 @@ -5482,13 +5486,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
  
   if (!lane)
   lane = ironlake_get_lanes_required(target_clock, link_bw,
 -intel_crtc-bpp);
 +intel_crtc-config.pipe_bpp);
  
   intel_crtc-fdi_lanes = lane;
  
   if (intel_crtc-config.pixel_multiplier  1)
   link_bw *= intel_crtc-config.pixel_multiplier;
 - intel_link_compute_m_n(intel_crtc-bpp, lane, target_clock, link_bw, 
 m_n);
 + intel_link_compute_m_n(intel_crtc-config.pipe_bpp, lane, target_clock,
 +link_bw, m_n);
  
   I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
   I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
 @@ -5651,8 +5656,10 @@

Re: [Intel-gfx] [PATCH 06/13] drm/i915: add pipe_config-has_pch_encoder

On Wed, Mar 27, 2013 at 6:06 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 +   if (HAS_PCH_SPLIT(dev)  !HAS_DDI(dev)  !is_cpu_edp(intel_dp))
 +   pipe_config-has_pch_encoder = true;
 +

 This could just do
  if (intel_dp-is_pch_edp)
 pipe_config-has_pch_encoder = true;
 right?  Since we cover the other cases in dp_init_connector?

That would give you two wrong case currently:
- hsw port D eDP would be marked as pch port
- any pch non-eDP DP ports would not be marked as pch ports

The ugly thing with this patch here is that this property is actually
fixed to the encoder, but I dynamically compute it in compute_config.
We have a few other such cases (e.g. the cpu transcoder for edp on
hsw). But I've figured there's no point in adding something clever,
which then updates the pipe_config according to connected encoders
with data structures ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/13] drm/i915: add pipe_config-timings_set

On Wed, 27 Mar 2013 18:06:44 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 On Wed, Mar 27, 2013 at 5:59 PM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  @@ -203,6 +201,10 @@ struct intel_connector {
   struct intel_crtc_config {
struct drm_display_mode requested_mode;
struct drm_display_mode adjusted_mode;
  + /* This flag must be set by the encoder's compute_config callback if 
  it
  +  * changes the crtc timings in the mode to prevent the crtc fixup 
  from
  +  * overwriting them.  Currently only lvds needs that. */
  + bool timings_set;
 
  The compute_config function could actually use some kdoc instead of
  putting it over the timings_set function.  It'll need to be expanded to
  cover all the pipe_config bits eventually, what they mean and when they
  should be set.
 
 Now I very much like to claim the opposite, but this isn't designed
 but very much organically grown code. So imo documentation doesn't
 make too much sense before things settle a bit more (the auto fdi link
 dither at the end will introduce quite a bit of fun still ...).
 
 I've promised though in my pipe_config intro a few weeks ago that I'll
 create a nice blog post and doc patch once the basic stuff is settled.
 I still intend to deliver on that. Is that good enough?

I guess so... incrementally adding to the compute_config kdoc with the
new pipe_config bits as added is too much rebase pain?

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw

On Wed, 27 Mar 2013 00:44:58 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 The procedure has now 3 steps:
 
 1. Compute the bpp that the plane will output, this is done in
pipe_config_set_bpp and stored into pipe_config-pipe_bpp. Also,
this function clamps the pipe_bpp to whatever limit the EDID of any
connected output specifies.
 2. Adjust the pipe_bpp in the encoder and crtc functions, according to
whatever constraints there are.
 3. Decide whether to use dither by comparing the stored plane bpp with
computed pipe_bpp.
 
 There are a few slight functional changes in this patch:
 - LVDS connector are now also going through the EDID clamping. But in
   a 2nd change we now unconditionally force the lvds bpc value - this
   shouldn't matter in reality when the panel setup is consistent, but
   better safe than sorry.
 - HDMI now forces the pipe_bpp to the selected value - I think that's
   what we actually want, since otherwise at least the pixelclock
   computations are wrong (I'm not sure whether the port would accept
   e.g. 10 bpc when in 12bpc mode). Contrary to the old code, we pick
   the next higher bpc value, since otherwise there's no way to make
   use of the 12 bpc mode (since the next patch will remove the 12bpc
   plane format, it doesn't exist).
 
 Both of these changes are due to the removal of the
 
   pipe_bpp = min(display_bpp, plane_bpp);
 
 statement.
 
 Another slight change is the reworking of the dp bpc code:
 - For the mode_valid callback it's sufficient to only check whether
   the mode would fit at the lowest bpc.
 - The bandwidth computation code is a bit restructured: It now walks
   all available bpp values in an outer loop and the codeblock that
   computes derived values (once a good configuration is found) has been
   moved out of the for loop maze. This is prep work to allow us to
   successively fall back on bpc values, and also correctly support bpc
   values != 8 or 6.
 
 v2: Rebased on top of Paulo Zanoni's little refactoring to use more
 drm dp helper functions.
 
 v3: Rebased on top of Jani's eDP bpp fix and Ville's limited color
 range work.
 
 v4: Remove the INTEL_MODE_DP_FORCE_6BPC #define, no longer needed.
 
 v5: Remove intel_crtc-bpp, too, and fix up the 12bpc check in the
 hdmi code. Also fixup the bpp check in intel_dp.c, it'll get reworked
 in a later patch though again.
 
 v6: Fix spelling in a comment.
 
 v7: Debug output improvements for the bpp computation.
 
 v8: Fixup 6bpc lvds check - dual-link and 8bpc mode are different
 things!
 
 v9: Reinstate the fix to properly ignore the firmware edp bpp ... this
 was lost in a rebase.
 
 v10: Both g4x and vlv lack 12bpc pipes, so don't enforce that we have
 that. Still unsure whether this is the way to go, but at least 6bpc
 for a 8bpc hdmi output seems to work.
 
 v11: And g4x/vlv also lack 12bpc hdmi support, so only support high
 depth on DP. Adjust the code.
 
 v12: Rebased.
 
 v13: Split out the introduction of pipe_config-dither|pipe_bpp, as
 requested from Jesse Barnes.
 
 v14: Split out the special 6BPC handling for DP, as requested by Jesse
 Barnes.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_ddi.c |   7 +-
  drivers/gpu/drm/i915/intel_display.c | 224 
 ---
  drivers/gpu/drm/i915/intel_hdmi.c|  13 ++
  drivers/gpu/drm/i915/intel_lvds.c|  12 ++
  4 files changed, 100 insertions(+), 156 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index 3d09df0..6c6b012 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -945,9 +945,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
   temp |= TRANS_MSA_12_BPC;
   break;
   default:
 - temp |= TRANS_MSA_8_BPC;
 - WARN(1, %d bpp unsupported by DDI function\n,
 -  intel_crtc-config.pipe_bpp);
 + BUG();
   }
   I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
   }
 @@ -983,8 +981,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc 
 *crtc)
   temp |= TRANS_DDI_BPC_12;
   break;
   default:
 - WARN(1, %d bpp unsupported by transcoder DDI function\n,
 -  intel_crtc-config.pipe_bpp);
 + BUG();
   }
  
   if (crtc-mode.flags  DRM_MODE_FLAG_PVSYNC)
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index b495629..6a60bf2 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4059,142 +4059,6 @@ static inline bool intel_panel_use_ssc(struct 
 drm_i915_private *dev_priv)
!(dev_priv-quirks  QUIRK_LVDS_SSC_DISABLE);
  }
  
 -/**
 - * intel_choose_pipe_bpp_dither - figure out what color depth the pipe 
 should send
 - *

Re: [Intel-gfx] [PATCH] drm/i915: wire up SDVO hpd support on cpt/ppt

2013-03-27 Thread Rodrigo Vivi

For a moment I confused CPT with LPT at bspec where 18 was reserved... but
now I found the correct one.

Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com




On Tue, Mar 26, 2013 at 6:38 PM, Daniel Vetter daniel.vet...@ffwll.chwrote:

 Now with Egbert Eich's hpd infrastructure rework merged this is dead
 simple. And we need this to make output detection work on SDVO - with
 the cleaned-up drm polling helpers outputs which claim to have hpd
 support are no longer polled.

 Now SDVO claims to do that, but it's not actually wired up. So just do
 it.

 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_irq.c |1 +
  drivers/gpu/drm/i915/i915_reg.h |2 ++
  2 files changed, 3 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_irq.c
 b/drivers/gpu/drm/i915/i915_irq.c
 index 43436e0..7cc18e2 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -46,6 +46,7 @@ static const u32 hpd_ibx[] = {

  static const u32 hpd_cpt[] = {
 [HPD_CRT] = SDE_CRT_HOTPLUG_CPT,
 +   [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG_CPT,
 [HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
 [HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT,
 [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT
 diff --git a/drivers/gpu/drm/i915/i915_reg.h
 b/drivers/gpu/drm/i915/i915_reg.h
 index 9703307..5e91fbb 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3589,7 +3589,9 @@
  #define SDE_PORTC_HOTPLUG_CPT  (1  22)
  #define SDE_PORTB_HOTPLUG_CPT  (1  21)
  #define SDE_CRT_HOTPLUG_CPT(1  19)
 +#define SDE_SDVOB_HOTPLUG_CPT  (1  18)
  #define SDE_HOTPLUG_MASK_CPT   (SDE_CRT_HOTPLUG_CPT |  \
 +SDE_SDVOB_HOTPLUG_CPT |\
  SDE_PORTD_HOTPLUG_CPT |\
  SDE_PORTC_HOTPLUG_CPT |\
  SDE_PORTB_HOTPLUG_CPT)
 --
 1.7.10.4

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx




-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw

On Wed, Mar 27, 2013 at 6:24 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 This looks good and seems to cover the bugs we've had here before.  My
 only concern is the one I mentioned before: on older chipsets we could
 dither between plane, pipe, *and* port.  Nowadays the pipe always does
 it.

 So in the old LVDS case it would be cool if someone could test the
 difference.  The LVDS port may do a better job on 6bpc panels than the
 pipe...

I've considered this again, and it should fit neatly into the existing
framework. If we want to use the dither on the lvds port on those
platforms, but keep dithering on the pipe for e.g. DP we could switch
lvds_compute_config to not clamp the bpp and then just enable the port
dithering if config.pipe_bpp != 18.

There's some further patches which unify this a bit, I guess we can
discuss this a bit more once I resend them.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: wire up SDVO hpd support on cpt/ppt

On Wed, Mar 27, 2013 at 03:46:15PM -0300, Rodrigo Vivi wrote:
 For a moment I confused CPT with LPT at bspec where 18 was reserved... but
 now I found the correct one.
 
 Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com
Queued for -next, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

2013-03-27 Thread Alexander van Heukelum

Hi Al,

Hans de Bruin found a regression due to one of your changes. I asked him to 
test a fix and he reported back that it worked. (Thanks!) Can you see if you 
agree with the fix? Patch is attached due to webmail...

Greetings,
Alexander
From 961a1b130aa79acb54f556a0accfcc643d1d3ed1 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum heuke...@fastmail.fm
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use asmlinkage calling convention

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the asmlinkage calling convention.

The regression was reported and pinpointed by Hans de Bruin:
 commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
 somehow breaks the colors when I play 'civilization I' under xdosemu.
 During the intro of the game something the colors get messed up. When
 the game begins the grass of the earth is red. Reverting the commit
 fixes the problem.

And he tested the patch too:
 Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin jmdebr...@xmsnet.nl
Signed-off-by: Alexander van Heukelum heuke...@fastmail.fm
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c   | 6 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..a245b88 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage int sys_vm86old(struct vm86_struct __user *);
+asmlinkage int sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..7f72807 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+asmlinkage int sys_vm86old(struct vm86_struct __user *v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+asmlinkage int sys_vm86(unsigned long cmd, unsigned long arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: keep backlight_level and backlight device brightness in sync

On Mon, Mar 25, 2013 at 02:56:39PM +0200, Jani Nikula wrote:
 
 Any comments on these two patches?

Both queued for -next, thanks for the patches. That means though that
you're volunteered for more cleanup ;-)
-Daniel

 
 BR,
 Jani.
 
 
 On Tue, 12 Mar 2013, Jani Nikula jani.nik...@intel.com wrote:
  A single point of truth would be better than two, but achieving that would
  require more abstractions for CONFIG_BACKLIGHT_CLASS_DEVICE=n with not a
  whole lot of real benefits. Take the short route and just keep the
  backlight levels in sync. In particular, update backlight device brightness
  on opregion brightness changes.
 
  Signed-off-by: Jani Nikula jani.nik...@intel.com
  ---
   drivers/gpu/drm/i915/intel_panel.c |   11 +--
   1 file changed, 9 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_panel.c 
  b/drivers/gpu/drm/i915/intel_panel.c
  index a3730e0..725d726 100644
  --- a/drivers/gpu/drm/i915/intel_panel.c
  +++ b/drivers/gpu/drm/i915/intel_panel.c
  @@ -287,6 +287,9 @@ void intel_panel_set_backlight(struct drm_device *dev, 
  u32 level)
  struct drm_i915_private *dev_priv = dev-dev_private;
   
  dev_priv-backlight_level = level;
  +   if (dev_priv-backlight)
  +   dev_priv-backlight-props.brightness = level;
  +
  if (dev_priv-backlight_enabled)
  intel_panel_actually_set_backlight(dev, level);
   }
  @@ -318,8 +321,12 @@ void intel_panel_enable_backlight(struct drm_device 
  *dev,
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
   
  -   if (dev_priv-backlight_level == 0)
  +   if (dev_priv-backlight_level == 0) {
  dev_priv-backlight_level = intel_panel_get_max_backlight(dev);
  +   if (dev_priv-backlight)
  +   dev_priv-backlight-props.brightness =
  +   dev_priv-backlight_level;
  +   }
   
  dev_priv-backlight_enabled = true;
  intel_panel_actually_set_backlight(dev, dev_priv-backlight_level);
  @@ -427,6 +434,7 @@ int intel_panel_setup_backlight(struct drm_connector 
  *connector)
   
  memset(props, 0, sizeof(props));
  props.type = BACKLIGHT_RAW;
  +   props.brightness = dev_priv-backlight_level;
  props.max_brightness = _intel_panel_get_max_backlight(dev);
  if (props.max_brightness == 0) {
  DRM_DEBUG_DRIVER(Failed to get maximum backlight value\n);
  @@ -443,7 +451,6 @@ int intel_panel_setup_backlight(struct drm_connector 
  *connector)
  dev_priv-backlight = NULL;
  return -ENODEV;
  }
  -   dev_priv-backlight-props.brightness = intel_panel_get_backlight(dev);
  return 0;
   }
   
  -- 
  1.7.9.5
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/13] drm/i915: convert DP autodither code to new infrastructure

On Wed, 27 Mar 2013 00:44:59 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 The old code only handled either 6bpc or 8bpc. Since it's easy to do,
 reorganize the code to be a bit more generic so that it can also handle
 10bpc and 12bpc. Note that we still start with 8bpc, so there's no
 functional change.
 
 Also, since we no don't need to compute the 6BPC flag in the mode_valid
 callback, we can consolidate things a bit. That requires though that
 the link bw computation is moved up in the compute_config callback.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c |   4 --
  drivers/gpu/drm/i915/intel_dp.c  | 103 
 ---
  drivers/gpu/drm/i915/intel_drv.h |   3 -
  3 files changed, 47 insertions(+), 63 deletions(-)
 

I had a harder time following this one (DP is just complex), but it
looks ok so far.  I'll feel better with lots of testing.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/13] drm/i915: clean up plane bpp confusion

On Wed, 27 Mar 2013 00:45:00 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 - There is no 16bpc linear color format in our hw. gen4+ has a 16 bpc
   float layout, but we don't really support it.
 - 10bpc is a gen4+ feature, fix up the support for it.
 - Update_plane should never see a wrong fb bpp value, BUG in the
   corresponding cases.
 
 v2: Rebase on top of Ville's plane pixel layout changes.
 
 v3: Actually drop the old gen4 check for 10bpc planes, spotted
 by Ville Syrjälä.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 20 
  1 file changed, 8 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 51557ba..bbf31aa 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2096,8 +2096,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, 
 struct drm_framebuffer *fb,
   dspcntr |= DISPPLANE_RGBX101010;
   break;
   default:
 - DRM_ERROR(Unknown pixel format 0x%08x\n, fb-pixel_format);
 - return -EINVAL;
 + BUG();
   }
  
   if (INTEL_INFO(dev)-gen = 4) {
 @@ -2190,8 +2189,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
   dspcntr |= DISPPLANE_RGBX101010;
   break;
   default:
 - DRM_ERROR(Unknown pixel format 0x%08x\n, fb-pixel_format);
 - return -EINVAL;
 + BUG();
   }
  
   if (obj-tiling_mode != I915_TILING_NONE)
 @@ -7372,21 +7370,19 @@ pipe_config_set_bpp(struct drm_crtc *crtc,
   bpp = 8*3;
   break;
   case 30:
 + if (INTEL_INFO(dev)-gen  4) {
 + DRM_DEBUG_KMS(10 bpc not supported on gen2/3\n);
 + return -EINVAL;
 + }
 +
   bpp = 10*3;
   break;
 - case 48:
 - bpp = 12*3;
 - break;
 + /* TODO: gen4+ supports 16 bpc floating point, too. */
   default:
   DRM_DEBUG_KMS(unsupported depth\n);
   return -EINVAL;
   }
  
 - if (fb-depth  24  !HAS_PCH_SPLIT(dev)) {
 - DRM_DEBUG_KMS(high depth not supported on gmch platforms\n);
 - return -EINVAL;
 - }
 -
   pipe_config-pipe_bpp = bpp;
  
   /* Clamp display bpp to EDID value */

You don't want to squash this into 8/13?  It looks ok.

Sorry about the 48; it's 16:16:16:16 ignoring alpha, so you end up with
48bpp and my backwards calc for bpc ignored alpha again and ended up at
12. :)

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

2013-03-27 Thread Alexander van Heukelum

On Wed, Mar 27, 2013, at 20:46, Al Viro wrote:
 On Wed, Mar 27, 2013 at 08:31:02PM +0100, Alexander van Heukelum wrote:
  Hi Al,
  
  Hans de Bruin found a regression due to one of your changes. I asked him to 
  test a fix and he reported back that it worked. (Thanks!) Can you see if 
  you agree with the fix? Patch is attached due to webmail...
 
 Use SYSCALL_DEFINE{1,2} for vm86_old and vm86, please.

Like this?

Greetings,
Alexander
From 450d86e6ad0a7d387cf706714c1fc030bb4b13a5 Mon Sep 17 00:00:00 2001
From: Alexander van Heukelum heuke...@fastmail.fm
Date: Tue, 26 Mar 2013 21:57:43 +0100
Subject: [PATCH] x86, vm86: fix VM86 syscalls: use SYSCALL_DEFINEx(...)

Commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
got rid of the pt_regs stub for sys_vm86old and sys_vm86. The functions
were, however, not changed to use the calling convention for syscalls.

[v2] Use SYSCALL_DEFINEx(...). Compiles to identical code.

The regression was reported and pinpointed by Hans de Bruin:
 commit 49cb25e9290 x86: 'get rid of pt_regs argument in vm86/vm86old'
 somehow breaks the colors when I play 'civilization I' under xdosemu.
 During the intro of the game something the colors get messed up. When
 the game begins the grass of the earth is red. Reverting the commit
 fixes the problem.

And he tested the patch too:
 Yep, the grass is green again.

Reported-and-tested-by: Hans de Bruin jmdebr...@xmsnet.nl
Signed-off-by: Alexander van Heukelum heuke...@fastmail.fm
---
 arch/x86/include/asm/syscalls.h | 4 ++--
 arch/x86/kernel/vm86_32.c   | 8 +---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h
index 6cf0a9c..5a0be0a 100644
--- a/arch/x86/include/asm/syscalls.h
+++ b/arch/x86/include/asm/syscalls.h
@@ -37,8 +37,8 @@ asmlinkage int sys_get_thread_area(struct user_desc __user *);
 unsigned long sys_sigreturn(void);
 
 /* kernel/vm86_32.c */
-int sys_vm86old(struct vm86_struct __user *);
-int sys_vm86(unsigned long, unsigned long);
+asmlinkage long sys_vm86old(struct vm86_struct __user *);
+asmlinkage long sys_vm86(unsigned long, unsigned long);
 
 #else /* CONFIG_X86_32 */
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 1cf5766..a67cb2b 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -33,6 +33,7 @@
 #include linux/capability.h
 #include linux/errno.h
 #include linux/interrupt.h
+#include linux/syscalls.h
 #include linux/sched.h
 #include linux/kernel.h
 #include linux/signal.h
@@ -48,7 +49,6 @@
 #include asm/io.h
 #include asm/tlbflush.h
 #include asm/irq.h
-#include asm/syscalls.h
 
 /*
  * Known problems:
@@ -202,7 +202,7 @@ out:
 static int do_vm86_irq_handling(int subfunction, int irqnumber);
 static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk);
 
-int sys_vm86old(struct vm86_struct __user *v86)
+SYSCALL_DEFINE1(vm86old, struct vm86_struct __user *, v86)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -227,11 +227,12 @@ int sys_vm86old(struct vm86_struct __user *v86)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(1, ret, v86);
 	return ret;
 }
 
 
-int sys_vm86(unsigned long cmd, unsigned long arg)
+SYSCALL_DEFINE2(vm86, unsigned long, cmd, unsigned long, arg)
 {
 	struct kernel_vm86_struct info; /* declare this _on top_,
 	 * this avoids wasting of stack space.
@@ -278,6 +279,7 @@ int sys_vm86(unsigned long cmd, unsigned long arg)
 	do_sys_vm86(info, tsk);
 	ret = 0;	/* we never return here */
 out:
+	asmlinkage_protect(2, ret, cmd, arg);
 	return ret;
 }
 
-- 
1.8.1.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/13] drm/i915: clean up pipe bpp confusion

On Wed, 27 Mar 2013 00:45:01 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 - gen4 and earlier (save for g4x) only really have a 8bpc pipe, with
   the possibility to dither to 6bpc using the panel fitter
 - g4x has hdmi, but no 12 bpc pipe ... !? Clamp hdmi accordingly.
 - TV/SDVO out are the only connectors available on platforms with
   a pipe bpp != 8, add code to force the pipe to 8bpc unconditionally.
 
 rant
 The dither handling on gmch platforms is one giant disaster. I'm hoping
 somewhat that vlv enabling will fix this up, but given that the 6bpc
 handling for edp was simply added with another quick hack, I don't have
 high hopes ...
 /rant
 
 v2: Neither vlv nor g4x have 12bpc pipes. Still set pipe_bpp to 12*3,
 but let the crtc code clamp things down to 10bpc on these platforms.
 
 v3: Fix a bpc vs. bpp mixup in the gen4 and earlier pipe_bpp limiter
 code.
 
 v4: Drop the hunk in intel_hdmi.c about g4x/vlv 12bpc, it was wrong.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c |  8 
  drivers/gpu/drm/i915/intel_sdvo.c|  3 +++
  drivers/gpu/drm/i915/intel_tv.c  | 14 --
  3 files changed, 19 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index bbf31aa..8ab7520 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3954,6 +3954,14 @@ static bool intel_crtc_compute_config(struct drm_crtc 
 *crtc,
   adjusted_mode-hsync_start == adjusted_mode-hdisplay)
   return false;
  
 + if ((IS_G4X(dev) || IS_VALLEYVIEW(dev))  pipe_config-pipe_bpp  10) {
 + pipe_config-pipe_bpp = 10*3; /* 12bpc is gen5+ */
 + } else if (INTEL_INFO(dev)-gen = 4  pipe_config-pipe_bpp  8) {
 + /* only a 8bpc pipe, with 6bpc dither through the panel fitter
 +  * for lvds. */
 + pipe_config-pipe_bpp = 8*3;
 + }
 +
   return true;
  }
  
 diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
 b/drivers/gpu/drm/i915/intel_sdvo.c
 index c6fbfd1..80f8680 100644
 --- a/drivers/gpu/drm/i915/intel_sdvo.c
 +++ b/drivers/gpu/drm/i915/intel_sdvo.c
 @@ -1048,6 +1048,9 @@ static bool intel_sdvo_compute_config(struct 
 intel_encoder *encoder,
   struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode;
   struct drm_display_mode *mode = pipe_config-requested_mode;
  
 + DRM_DEBUG_KMS(forcing bpc to 8 for SDVO\n);
 + pipe_config-pipe_bpp = 8*3;
 +
   if (HAS_PCH_SPLIT(encoder-base.dev))
   pipe_config-has_pch_encoder = true;
  
 diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
 index d808421..6673726 100644
 --- a/drivers/gpu/drm/i915/intel_tv.c
 +++ b/drivers/gpu/drm/i915/intel_tv.c
 @@ -905,11 +905,10 @@ intel_tv_mode_valid(struct drm_connector *connector,
  
  
  static bool
 -intel_tv_mode_fixup(struct drm_encoder *encoder,
 - const struct drm_display_mode *mode,
 - struct drm_display_mode *adjusted_mode)
 +intel_tv_compute_config(struct intel_encoder *encoder,
 + struct intel_crtc_config *pipe_config)
  {
 - struct intel_tv *intel_tv = enc_to_intel_tv(encoder);
 + struct intel_tv *intel_tv = enc_to_intel_tv(encoder-base);
   const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
  
   if (!tv_mode)
 @@ -918,7 +917,10 @@ intel_tv_mode_fixup(struct drm_encoder *encoder,
   if (intel_encoder_check_is_cloned(intel_tv-base))
   return false;
  
 - adjusted_mode-clock = tv_mode-clock;
 + pipe_config-adjusted_mode.clock = tv_mode-clock;
 + DRM_DEBUG_KMS(forcing bpc to 8 for TV\n);
 + pipe_config-pipe_bpp = 8*3;
 +
   return true;
  }
  
 @@ -1485,7 +1487,6 @@ out:
  }
  
  static const struct drm_encoder_helper_funcs intel_tv_helper_funcs = {
 - .mode_fixup = intel_tv_mode_fixup,
   .mode_set = intel_tv_mode_set,
  };
  
 @@ -1620,6 +1621,7 @@ intel_tv_init(struct drm_device *dev)
   drm_encoder_init(dev, intel_encoder-base, intel_tv_enc_funcs,
DRM_MODE_ENCODER_TVDAC);
  
 + intel_encoder-compute_config = intel_tv_compute_config;
   intel_encoder-enable = intel_enable_tv;
   intel_encoder-disable = intel_disable_tv;
   intel_encoder-get_hw_state = intel_tv_get_hw_state;

I had to double check this against 9/13... I guess the order will be
clearer with the actual code as opposed to patches.

But won't these override the pipe_config bpp we set in
pipe_config_set_bpp()?  Why bother setting it there if each of the
encoders will set it themselves, and the real comparison is against
the plane bpp?  And doesn't that mean we'd need to set pipe_config-bpp
in the DP version too?

Maybe I've been looking at this too hard and I've just confused
myself...

-- 
Jesse Barnes, Intel Open Source

Re: [Intel-gfx] [PATCH 12/13] drm/i915: clean up pipe bpp confusion

On Wed, Mar 27, 2013 at 10:28 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 I had to double check this against 9/13... I guess the order will be
 clearer with the actual code as opposed to patches.

 But won't these override the pipe_config bpp we set in
 pipe_config_set_bpp()?  Why bother setting it there if each of the
 encoders will set it themselves, and the real comparison is against
 the plane bpp?  And doesn't that mean we'd need to set pipe_config-bpp
 in the DP version too?

The pipe_bpp we set from the planes is the proposed one, used when
nothing else overrides it. Then encoders can come around and add in
their opinion about what's possible. Note that encoders want to know
which pipe_bpp is the proposed one (from looking just at the plane) to
make their own decision. E.g. hdmi wants to updither 10bpc to 12bpc
(if possible) since it doesn't support 10bpc natively. Whereas DP only
ever down-dithers.

This way we gain a notch more flexibility in handling bpp.

My auto-fdi dither work which is based on top of this goes one step
further and checks (after plane/pipe/encoder all had their say)
whether it actually fits into the fdi link. If it doesn't fit it tries
to dither down. If that's possible we'll restart the pipe_config
arbitrage, but with the new proposed pipe_bpp plus a flag telling
everyone that they'll get shot if they try to increase bw
requirements.

 Maybe I've been looking at this too hard and I've just confused
 myself...

Maybe it's a bit overdesigned ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915: fold wait_for_atomic_us into wait_for_atomic

Since

commit bcf9dcc1e6269fac674e41f25d007ff75f76e840
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Sun Jul 15 09:42:38 2012 +0100

drm/i915: Workaround hang with BSD and forcewake on SandyBridge

and

commit 0cc2764cc4a4bd73df55f8893c871778cf7ddd0f
Author: Ben Widawsky b...@bwidawsk.net
Date:   Sat Sep 1 22:59:48 2012 -0700

drm/i915: use cpu_relax() in wait_for_atomic

these two macros are essentially the same, so unify them. We keep the
_us version since it's a nice documentation for smaller timeouts.

Cc: Jack Winter j...@alchemy.lu
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_drv.h | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 54bc2ea..c8c1979 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -50,21 +50,10 @@
ret__;  \
 })
 
-#define wait_for_atomic_us(COND, US) ({ \
-   unsigned long timeout__ = jiffies + usecs_to_jiffies(US);   \
-   int ret__ = 0;  \
-   while (!(COND)) {   \
-   if (time_after(jiffies, timeout__)) {   \
-   ret__ = -ETIMEDOUT; \
-   break;  \
-   }   \
-   cpu_relax();\
-   }   \
-   ret__;  \
-})
-
 #define wait_for(COND, MS) _wait_for(COND, MS, 1)
 #define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
+#define wait_for_atomic_us(COND, US) _wait_for((COND), \
+  usecs_to_jiffies((US)), 0)
 
 #define KHz(x) (1000*x)
 #define MHz(x) KHz(1000*x)
-- 
1.7.11.7

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: fix up _wait_for macro

As Thomas Gleixner spotted, it's rather horrible racy:
- We can miss almost a full tick, so need to compensate by 1 jiffy.
- We need to re-check the condition when having timed-out, since a
  the last check could have been before the timeout expired. E.g. when
  we've been preempted or a long irq happened.

Cc: Thomas Gleixner t...@linutronix.de
Reported-by: Jack Winter j...@alchemy.lu
Cc: Jack Winter j...@alchemy.lu
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_drv.h | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c8c1979..9dcae4e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -33,12 +33,21 @@
 #include drm/drm_fb_helper.h
 #include drm/drm_dp_helper.h
 
+/**
+ * _wait_for - magic (register) wait macro
+ *
+ * Does the right thing for modeset paths when run under kdgb or similar atomic
+ * contexts. Note that it's important that we check the condition again after
+ * having timed out, since the timeout could be due to preemption or similar 
and
+ * we've never had a chance to check the condition before the timeout.
+ */
 #define _wait_for(COND, MS, W) ({ \
-   unsigned long timeout__ = jiffies + msecs_to_jiffies(MS);   \
+   unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1;   \
int ret__ = 0;  \
while (!(COND)) {   \
if (time_after(jiffies, timeout__)) {   \
-   ret__ = -ETIMEDOUT; \
+   if (!(COND))\
+   ret__ = -ETIMEDOUT; \
break;  \
}   \
if (W  drm_can_sleep())  {\
-- 
1.7.11.7

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/13] drm/i915: clean up pipe bpp confusion

On Wed, 27 Mar 2013 23:41:55 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 On Wed, Mar 27, 2013 at 10:28 PM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  I had to double check this against 9/13... I guess the order will be
  clearer with the actual code as opposed to patches.
 
  But won't these override the pipe_config bpp we set in
  pipe_config_set_bpp()?  Why bother setting it there if each of the
  encoders will set it themselves, and the real comparison is against
  the plane bpp?  And doesn't that mean we'd need to set pipe_config-bpp
  in the DP version too?
 
 The pipe_bpp we set from the planes is the proposed one, used when
 nothing else overrides it. Then encoders can come around and add in
 their opinion about what's possible. Note that encoders want to know
 which pipe_bpp is the proposed one (from looking just at the plane) to
 make their own decision. E.g. hdmi wants to updither 10bpc to 12bpc
 (if possible) since it doesn't support 10bpc natively. Whereas DP only
 ever down-dithers.
 
 This way we gain a notch more flexibility in handling bpp.
 
 My auto-fdi dither work which is based on top of this goes one step
 further and checks (after plane/pipe/encoder all had their say)
 whether it actually fits into the fdi link. If it doesn't fit it tries
 to dither down. If that's possible we'll restart the pipe_config
 arbitrage, but with the new proposed pipe_bpp plus a flag telling
 everyone that they'll get shot if they try to increase bw
 requirements.
 
  Maybe I've been looking at this too hard and I've just confused
  myself...
 
 Maybe it's a bit overdesigned ;-)

Ok it makes some sense... though maybe if we passed down the plane bpp
directly we'd be able to avoid some of the re-calc stuff in your FDI
dither patch.

We can always improve it after it lands and becomes clearer.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/13] drm/i915: clean up pipe bpp confusion

On Wed, Mar 27, 2013 at 04:13:13PM -0700, Jesse Barnes wrote:
 On Wed, 27 Mar 2013 23:41:55 +0100
 Daniel Vetter daniel.vet...@ffwll.ch wrote:
 
  On Wed, Mar 27, 2013 at 10:28 PM, Jesse Barnes jbar...@virtuousgeek.org 
  wrote:
   I had to double check this against 9/13... I guess the order will be
   clearer with the actual code as opposed to patches.
  
   But won't these override the pipe_config bpp we set in
   pipe_config_set_bpp()?  Why bother setting it there if each of the
   encoders will set it themselves, and the real comparison is against
   the plane bpp?  And doesn't that mean we'd need to set pipe_config-bpp
   in the DP version too?
  
  The pipe_bpp we set from the planes is the proposed one, used when
  nothing else overrides it. Then encoders can come around and add in
  their opinion about what's possible. Note that encoders want to know
  which pipe_bpp is the proposed one (from looking just at the plane) to
  make their own decision. E.g. hdmi wants to updither 10bpc to 12bpc
  (if possible) since it doesn't support 10bpc natively. Whereas DP only
  ever down-dithers.
  
  This way we gain a notch more flexibility in handling bpp.
  
  My auto-fdi dither work which is based on top of this goes one step
  further and checks (after plane/pipe/encoder all had their say)
  whether it actually fits into the fdi link. If it doesn't fit it tries
  to dither down. If that's possible we'll restart the pipe_config
  arbitrage, but with the new proposed pipe_bpp plus a flag telling
  everyone that they'll get shot if they try to increase bw
  requirements.
  
   Maybe I've been looking at this too hard and I've just confused
   myself...
  
  Maybe it's a bit overdesigned ;-)
 
 Ok it makes some sense... though maybe if we passed down the plane bpp
 directly we'd be able to avoid some of the re-calc stuff in your FDI
 dither patch.
 
 We can always improve it after it lands and becomes clearer.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

Slurped them all into dinq, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] add alias for ville