Re: [Intel-gfx] [PATCH] drm/i915: Always use kref tracking for all contexts.

2014-04-11 Thread Ben Widawsky
On Wed, Apr 09, 2014 at 09:07:36AM +0100, Chris Wilson wrote:
 If we always initialize kref for the context, even if we are using fake
 contexts for hangstats when there is no hw support, we can forgo the
 dance to dereference the ctx-obj and inspect whether we are permitted
 to use kref inside i915_gem_context_reference() and _unreference().
 
 My ulterior motive here is to improve the debugging of a use-after-free
 of ctx-obj. This patch avoids the dereference here and instead forces
 the assertion checks associated with kref.
 
 v2: Refactor the fake contexts to being even more like the real
 contexts, so that there is much less duplicated and special case code.
 
 v3: Tweaks.
 v4: Tweaks, minor.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76671
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Tested-by: lu hua huax...@intel.com
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 Cc: Mika Kuoppala mika.kuopp...@intel.com

I couldn't spot any problems, but I got a bit lazier with each review
since v2.

Reviewed-by: Ben Widawsky b...@bwidawsk.net

 ---
  drivers/gpu/drm/i915/i915_drv.h|   8 +-
  drivers/gpu/drm/i915/i915_gem.c|   2 +-
  drivers/gpu/drm/i915/i915_gem_context.c| 234 
 -
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   2 +-
  4 files changed, 101 insertions(+), 145 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 533bd8f6a5b1..0557bd92b26b 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2276,20 +2276,18 @@ int i915_gem_context_open(struct drm_device *dev, 
 struct drm_file *file);
  int i915_gem_context_enable(struct drm_i915_private *dev_priv);
  void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
  int i915_switch_context(struct intel_ring_buffer *ring,
 - struct drm_file *file, struct i915_hw_context *to);
 + struct i915_hw_context *to);
  struct i915_hw_context *
  i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
  void i915_gem_context_free(struct kref *ctx_ref);
  static inline void i915_gem_context_reference(struct i915_hw_context *ctx)
  {
 - if (ctx-obj  HAS_HW_CONTEXTS(ctx-obj-base.dev))
 - kref_get(ctx-ref);
 + kref_get(ctx-ref);
  }
  
  static inline void i915_gem_context_unreference(struct i915_hw_context *ctx)
  {
 - if (ctx-obj  HAS_HW_CONTEXTS(ctx-obj-base.dev))
 - kref_put(ctx-ref, i915_gem_context_free);
 + kref_put(ctx-ref, i915_gem_context_free);
  }
  
  static inline bool i915_gem_context_is_default(const struct i915_hw_context 
 *c)
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index eb5cf077c626..4368437458fd 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2819,7 +2819,7 @@ int i915_gpu_idle(struct drm_device *dev)
  
   /* Flush everything onto the inactive list. */
   for_each_ring(ring, dev_priv, i) {
 - ret = i915_switch_context(ring, NULL, ring-default_context);
 + ret = i915_switch_context(ring, ring-default_context);
   if (ret)
   return ret;
  
 diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
 b/drivers/gpu/drm/i915/i915_gem_context.c
 index 30b355afb362..f77b4c126465 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -96,9 +96,6 @@
  #define GEN6_CONTEXT_ALIGN (6410)
  #define GEN7_CONTEXT_ALIGN 4096
  
 -static int do_switch(struct intel_ring_buffer *ring,
 -  struct i915_hw_context *to);
 -
  static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
  {
   struct drm_device *dev = ppgtt-base.dev;
 @@ -185,13 +182,15 @@ void i915_gem_context_free(struct kref *ctx_ref)
  typeof(*ctx), ref);
   struct i915_hw_ppgtt *ppgtt = NULL;
  
 - /* We refcount even the aliasing PPGTT to keep the code symmetric */
 - if (USES_PPGTT(ctx-obj-base.dev))
 - ppgtt = ctx_to_ppgtt(ctx);
 + if (ctx-obj) {
 + /* We refcount even the aliasing PPGTT to keep the code 
 symmetric */
 + if (USES_PPGTT(ctx-obj-base.dev))
 + ppgtt = ctx_to_ppgtt(ctx);
  
 - /* XXX: Free up the object before tearing down the address space, in
 -  * case we're bound in the PPGTT */
 - drm_gem_object_unreference(ctx-obj-base);
 + /* XXX: Free up the object before tearing down the address 
 space, in
 +  * case we're bound in the PPGTT */
 + drm_gem_object_unreference(ctx-obj-base);
 + }
  
   if (ppgtt)
   kref_put(ppgtt-ref, ppgtt_release);
 @@ -232,40 +231,40 @@ __create_hw_context(struct drm_device *dev,
   return ERR_PTR(-ENOMEM);
  
   kref_init(ctx-ref);
 - ctx-obj = 

Re: [Intel-gfx] [PATCH] BDW swizzling

2014-04-11 Thread Ben Widawsky
On Thu, Apr 10, 2014 at 03:50:35PM -0700, Ben Widawsky wrote:
 On Thu, Apr 10, 2014 at 06:51:50PM +0100, Damien Lespiau wrote:
[snip]

  
  Do you know if you have a configuration where we try to swizzle? If yes
  and tests/gem_tiled_pread is passing that would give us a nice bit of
  information. (which of course can be tried by the next person with time
  to do so).
  
 
 If you get it wrong, it looks really obvious. Swizzling is *supposed* to
 be one of those transparent things (I thought). What follows can be
 entirely wrong, it's mostly from memory and a brief conversation with
 Art.
 
 There are 3 places that care about swizzling:
 1. The memory/DRAM controller
 2. The displayer interface to memory
 3. The GAM arbiter (generic interface to memory)
 
 It may or may not be talking about the same type of swizzling (bit) in
 all cases. The important thing, and what I have observed, is that the
 GAM and DE match on how things are swizzled. Otherwise we render/blit to
 a surface and it gets [de]swizzled when it's displayed. I never measured
 performance for setting both to 0, instead of 1.
 
 The part that's confused me has always been why we are supposed to
 program it based on #1. The way the DRAM controller decides to lay out
 the physical rows/banks etc. shouldn't matter as long as everyone goes
 through the same DRAM controller. It should just be transparent linear
 RAM. In other words, the comment about how we need to program the
 swizzle based on the DRAM controller never quite made sense to me. It's
 also possible if you enable one, you shouldn't/should enable another
 since compounding swizzling may be self-defeating. Dunno - so maybe your
 patch helps, maybe it hurts.
 
 Art suggested that the swizzling in GAM and DE predate the DRAM
 swizzler.

My bad. I just read the actual patch's commit message. Seems like you
knew all this already. Feel free to ignore me. I'll try to read both
before responding, next time.

-- 
Ben Widawsky, 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 0/4] Reduce intel_display.c

2014-04-11 Thread Jani Nikula
On Fri, 11 Apr 2014, Ben Widawsky b...@bwidawsk.net wrote:
 On Wed, Apr 09, 2014 at 06:44:29PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 Hi
 
 We always talk about how intel_display.c is a giant file and how we would 
 like
 to reduce it, so this is my attempt. Currently the file has 12090 lines, and
 after my patch series it has 8850 lines.
 
 I don't know if right now is the appropriate time to merge patches like 
 this. I
 don't remember seeing too many patches on the list touching 
 cursor/fdi/eld/pll
 functions, but I know there is never an appropriate time for huge changes.
 
 Also, this change will obviously make the lives of people who backport our
 patches more complicated. So if we don't want this series at all, feel free 
 to
 NACK it.

 I am only responding because it seems nobody else really said much. I
 never touch this code, and I shouldn't be the authority. I really
 quickly glanced at the patches.

 1. +LOC: It sucks that you ended up adding 220 lines. I assume half of it is 
 the
 copyright header, but still, considering there are no actual refactors,
 cleanups, or functional changes - adding lines makes me unhappy.

 2. necessary? I personally haven't heard from anyone that we need to shrink
 intel_display.c (again, I am the furthest from being an expert). I doubt
 anyone isn't using some form of tags, or grep to navigate anyway. My
 problem has never been the file size itself, but just the structure of
 the display code interacting with the core KMS was hard to follow.

 3. conflicts: Like you said, it's likely nobody touches this code, but we 
 should
 keep in mind we do have several people working on older branches, and
 this kind of thing makes any sort of backport hard.

 On the other hand:
 1. If more than one person finds the results more readable/consumable, I
 think it's worth it, and probably mostly justifies doing it. You've also
 shrunk the file by quite a bit, so it's somewhat useful churn.

 2. intel_pll.c sounds like a good idea

I'm in favour of reducing the size of intel_display.c. I did not look at
the patches though, because I've sent a few patches to this effect in
the past (limits/pll and quirks at least), but they were stalled because
they were in the collision course with the Grand Plans Daniel has. So
Daniel, I expect you to chime in on this one too. ;)

To reduce the conflict/backport pains, might be good to do this spread
out over a few releases instead of everything at once. *shrug*.

BR,
Jani.






 
 I also didn't really know what kind of changes I needed to do to the file
 headers, so I just copied the header from intel_display.c, kept Eric's name 
 and
 added a 2014 to Intel's copyright. I am not a lawyer and this may be not 
 the
 best thing to do, so please tell me the correct approach here :)
 
 There are also some things that we might want to migrate from intel_ddi.c to
 intel_pll.c, but I'll leave this to another patch.
 
 Also, feel free to propose better ways to split intel_display.c.
 
 Thanks,
 Paulo
 
 Paulo Zanoni (4):
   drm/i915: extract intel_eld.c from intel_display.c
   drm/i915: extract intel_cursor.c from intel_display.c
   drm/i915: extract intel_fdi.c from intel_display.c
   drm/i915: extract intel_pll.c from intel_display.c
 
  drivers/gpu/drm/i915/Makefile|4 +
  drivers/gpu/drm/i915/intel_cursor.c  |  357 
  drivers/gpu/drm/i915/intel_ddi.c |  142 +-
  drivers/gpu/drm/i915/intel_display.c | 3622 
 ++
  drivers/gpu/drm/i915/intel_drv.h |  143 +-
  drivers/gpu/drm/i915/intel_eld.c |  355 
  drivers/gpu/drm/i915/intel_fdi.c |  959 +
  drivers/gpu/drm/i915/intel_panel.c   |   36 +
  drivers/gpu/drm/i915/intel_pll.c | 1779 +
  9 files changed, 3808 insertions(+), 3589 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/intel_cursor.c
  create mode 100644 drivers/gpu/drm/i915/intel_eld.c
  create mode 100644 drivers/gpu/drm/i915/intel_fdi.c
  create mode 100644 drivers/gpu/drm/i915/intel_pll.c
 
 -- 
 1.9.0
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

-- 
Jani Nikula, 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/4] drm/i915: Invalidate our pages under memory pressure

2014-04-11 Thread Chris Wilson
On Tue, Mar 25, 2014 at 01:23:06PM +, Chris Wilson wrote:
 Try to flush out dirty pages into the swapcache (and from there into the
 swapfile) when under memory pressure and forced to drop GEM objects from
 memory. In effect, this should just allow us to discard unused pages for
 memory reclaim and to start writeback earlier.
 
 v2: Hugh Dickins warned that explicitly starting writeback from
 shrink_slab was prone to deadlocks within shmemfs.
 
 Cc: Hugh Dickins hu...@google.com
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Good news! QA have declared that this series really does prevent the
random OOM where we have completely unused swap. So all it needs is
someone brave enough to review.
-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 4/4] drm/i915: Invalidate our pages under memory pressure

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 09:30:20AM +0100, Chris Wilson wrote:
 On Tue, Mar 25, 2014 at 01:23:06PM +, Chris Wilson wrote:
  Try to flush out dirty pages into the swapcache (and from there into the
  swapfile) when under memory pressure and forced to drop GEM objects from
  memory. In effect, this should just allow us to discard unused pages for
  memory reclaim and to start writeback earlier.
  
  v2: Hugh Dickins warned that explicitly starting writeback from
  shrink_slab was prone to deadlocks within shmemfs.
  
  Cc: Hugh Dickins hu...@google.com
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 
 Good news! QA have declared that this series really does prevent the
 random OOM where we have completely unused swap. So all it needs is
 someone brave enough to review.

Awesome work.

Do we need the additional patch you've just posted to improve the
writeback stalling after calling shrinkers too, or is that not required?

wrt reviewers I'm poking people to get this done from our side. For
merging an ack on the mm pieces would be good so that I can pull it in
through drm-intel trees. My plan is to merge it in 3.16 if it all checks
out.

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 4/4] drm/i915: Invalidate our pages under memory pressure

2014-04-11 Thread Chris Wilson
On Fri, Apr 11, 2014 at 10:38:25AM +0200, Daniel Vetter wrote:
 On Fri, Apr 11, 2014 at 09:30:20AM +0100, Chris Wilson wrote:
  On Tue, Mar 25, 2014 at 01:23:06PM +, Chris Wilson wrote:
   Try to flush out dirty pages into the swapcache (and from there into the
   swapfile) when under memory pressure and forced to drop GEM objects from
   memory. In effect, this should just allow us to discard unused pages for
   memory reclaim and to start writeback earlier.
   
   v2: Hugh Dickins warned that explicitly starting writeback from
   shrink_slab was prone to deadlocks within shmemfs.
   
   Cc: Hugh Dickins hu...@google.com
   Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  
  Good news! QA have declared that this series really does prevent the
  random OOM where we have completely unused swap. So all it needs is
  someone brave enough to review.
 
 Awesome work.
 
 Do we need the additional patch you've just posted to improve the
 writeback stalling after calling shrinkers too, or is that not required?

That looks to be required (or at least I hope it provokes the mm gods
into doing something sensible) for a different problem. However, that
test is still behaving strangely (inactive_anon =~ 2x shmem, it should
be almost equal) as it appears that there is severe overallocation, or a
leak.

But that we can generate massive amounts of writeback from our shrinker
which may not be cleared in time for the allocation to succeed is a
problem (addressed by that mm patch).
-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 3/6] drm/i915: Add support for stealing purgable stolen pages

2014-04-11 Thread Daniel Vetter
On Thu, Apr 10, 2014 at 10:12:39AM +, Gupta, Sourab wrote:
 On Wed, 2014-04-09 at 13:06 +, Daniel Vetter wrote:
  On Tue, Apr 08, 2014 at 06:53:03AM +, Gupta, Sourab wrote:
   On Tue, 2014-04-08 at 06:45 +, Chris Wilson wrote:
On Tue, Apr 08, 2014 at 04:32:02AM +, Gupta, Sourab wrote:
 Hi Rodrigo,
 In this patch, while freeing the purgeable stolen object, the memory
 node also has to be freed, so as to make space for new object. We need
 to call drm_mm_remove_node while freeing obj.
 
 The below modification patch was floated earlier for this purpose:
 http://lists.freedesktop.org/archives/intel-gfx/2014-March/041282.html

Right, I have a v2 locally with the fix you identified.
-Chris

   Ok, Thanks Chris.
  
  I'd really prefer if someone would pick up all the
  stolen/create2_ioctl/whatever patches, pack them up into a polished
  series, add the testcases and submit this all for review and merging.
  
  Otherwise this will linger forever and we'll get nowhere. Chris seems
  swamped with other stuff, so Sourab could you please take a look at this?
  
  Please check with your manager that you have sufficient bandwidth to pull
  this through.
  
 
 I'll be on vacation from next week, so I'll be able to gauge this better
 after coming back.
 Nevertheless, I have some questions regarding the expectation of
 userspace code changes required for these patches (i.e. libdrm changes
 and igt testcases)
 
 1) For libdrm , I am assuming, a counterpart of
 drm_intel_gem_bo_alloc_tiled() function would call the create2 ioctl and
 take in the parameters needed. 
 Should the caching of objects from libdrm need to take care of both the
 placement domains seperately (as in different sets of bo buckets)?
 Should libdrm be transparent to all the combinations of different
 parameters being passed by user or should the prohibited combinations be
 disallowed from libdrm side?

I'm not sure whether we need a cache implemented in libdrm. Since stolen
objects are fairly special it's probably easier to just have a simple
linear cache tailor-made in the respective UMD. So just exposing
create2_ioctl should be good enough.

 2) For the igt, since we have a lot of parameters exposed to user, the
 number of subtests required may be huge and still they may not test out
 everything. 
 So, Is the expectation here to have exhaustive test cases for all the
 parameters (tiling/cache/domain/madvise/offset etc.) going in as input
 to the create2 ioctl?
 For eg. let us say we are going to check the render copy operation of
 src and dest bo's. Do we need to provide all possible combinations of
 different (create2 ioctl) input parameters to these src and dest bo's
 and then run the render copy test for all these combinations.
 Any guiding pointers from your side as to how we may go about the igt
 testcases?

At a high-level there's two parts for igt tests:
- First is functional tests, where we try to make sure that the feature
  actually works. I.e. allocate some stolen memory and then do something
  with it, making sure that data access for the gpu and similar things
  work. For this we just want some reasonable base coverage so that when
  we hit a bug somewhere it's easy to extend the testcase to cover that
  bug with a specific subtest.

- Then there's interface testing. kernel/userspace is a trust barrier, so
  we need to make sure that evil userspace can't make the kernel crash
  with some crazy invalid combination of flags or operations (like create
  a stolen object and then try to mmap it). Since this is security
  relevant and also since we can't ever change established userspace ABI I
  want full coverage of all cases. But this is just about detecting abuse
  correctly, no functional tests here.

For details see my two blog posts on the topic:

http://blog.ffwll.ch/2013/11/botching-up-ioctls.html

http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html

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] drm/i915/bdw: Use timeout mode for RC6 on bdw

2014-04-11 Thread Daniel Vetter
On Thu, Apr 10, 2014 at 08:29:55PM +, O'Rourke, Tom wrote:
   Higher RC6 residency is observed using timeout mode instead of EI
   mode.  This applies to Broadwell only.
   The difference is particularly noticeable with video playback.
  
   Issue: VIZ-3778
   Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d
   Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com
 
  How recent a nightly branch have you used to obtain these results?
  Chris just fixed some serious bugs in the gpu booster logic which
  would have affected all intermediate workloads.
  -Daniel
 
 He must not be using nightly if he has any BDW RC6 residency at all.
 
 [TOR:] Ben is correct.  I was testing mostly with a kernel for Android.
 I also tested with Ben's broadwell branch and saw similar improvement.

Ok I think it'd be good to have this when we actually merge/enable the
final pieces of the bdw rc6 code. Can you please work together with Ben to
make sure this patch isn't lost?

Or who is shepherding bdw rc6 nowadays?
-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/bdw: BDW swizzling in done by the memory controller

2014-04-11 Thread Daniel Vetter
On Thu, Apr 10, 2014 at 05:24:08PM +0100, Damien Lespiau wrote:
 Instead of needing to configure swizzling in 3 units (GAM, GT, DE), the
 memory controller is in charge of doing them on BDW. As a consequence
 all those swizzling bits are reserved. As specs put it:
 
   Before Gen8, there was a historical configuration control field to
   swizzle address bit[6] for in X/Y tiling modes. This was set in three
   different places: TILECTL[1:0], ARB_MODE[5:4], and
   DISP_ARB_CTL[14:13]
 
   For Gen8 the swizzle fields are all reserved, and the CPU's memory
   controller performs all address swizzling modifications.
 
 This also means that user space doesn't have to manually swizzle when
 accessing tiled buffers from the CPU, and so we always return
 I915_BIT_6_SWIZZLE_NONE from i915_gem_detect_bit_6_swizzle(), which
 short-circuits the initialization of the registers mentionned above in
 i915_gem_init_swizzling().
 
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com

Afaik the memory controller has always done the swizzling. What this pile
of bits controls is the _additional_ swizzling done to improve the access
pattern for 2d data, i.e. everything X and Y tiled. The theory of
operation is that the additional swizzling improves access patterns when
walking in Y direction on a surface.

And when we've last looked at this (well Chris) it seemed to indeed have
improved sampler performace. The downside is that it's a bit a pain for
userspace since essentially userspace has to deal with 4 tiling formats
instead of just 2, but we have all the code for that.

So not yet sold on the story here, at least for gen8/bdw. Apparently
people haven't screamed about this yet, so it probably works.
-Daniel

 ---
  drivers/gpu/drm/i915/i915_gem.c|  2 --
  drivers/gpu/drm/i915/i915_gem_tiling.c | 10 +-
  drivers/gpu/drm/i915/i915_reg.h|  1 -
  3 files changed, 9 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 85c9cf0..9032c1b 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -4325,8 +4325,6 @@ void i915_gem_init_swizzling(struct drm_device *dev)
   I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_SNB));
   else if (IS_GEN7(dev))
   I915_WRITE(ARB_MODE, _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_IVB));
 - else if (IS_GEN8(dev))
 - I915_WRITE(GAMTARBMODE, 
 _MASKED_BIT_ENABLE(ARB_MODE_SWIZZLE_BDW));
   else
   BUG();
  }
 diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
 b/drivers/gpu/drm/i915/i915_gem_tiling.c
 index cb150e8..a5ddf12 100644
 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
 +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
 @@ -91,7 +91,15 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
   uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
   uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
  
 - if (IS_VALLEYVIEW(dev)) {
 + if (INTEL_INFO(dev)-gen = 8) {
 + /*
 +  * On BDW+, the CPU memory controller performs all address
 +  * swizzling modifications. This condition also catches CHV,
 +  * where swizzling is not supported.
 +  */
 + swizzle_x = I915_BIT_6_SWIZZLE_NONE;
 + swizzle_y = I915_BIT_6_SWIZZLE_NONE;
 + } else if (IS_VALLEYVIEW(dev)) {
   swizzle_x = I915_BIT_6_SWIZZLE_NONE;
   swizzle_y = I915_BIT_6_SWIZZLE_NONE;
   } else if (INTEL_INFO(dev)-gen = 6) {
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 01c05af..faba21b 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -781,7 +781,6 @@ enum punit_power_well {
  #define   ARB_MODE_SWIZZLE_IVB   (15)
  #define GAMTARBMODE  0x04a08
  #define   ARB_MODE_BWGTLB_DISABLE (19)
 -#define   ARB_MODE_SWIZZLE_BDW   (11)
  #define RENDER_HWS_PGA_GEN7  (0x04080)
  #define RING_FAULT_REG(ring) (0x4094 + 0x100*(ring)-id)
  #define   RING_FAULT_GTTSEL_MASK (111)
 -- 
 1.8.3.1
 
 ___
 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] [v2] drm/i915/bdw: Add 42ms delay for IPS disable

2014-04-11 Thread Daniel Vetter
On Thu, Apr 10, 2014 at 11:38:21PM +, Runyan, Arthur J wrote:
 Ben explained some of the fine details of the code to me, and I'm happy.
 Reviewed-by: Art Runyan arthur.j.run...@intel.com
 
 
 From: Ben Widawsky benjamin.widaw...@linux.intel.com
 
 This is a requirement added to the spec. This patch will prevent
 persistent corruption on the display.
 
 v2: Make the wait before the vblank wait. (Art)
 Try to finish early by polling the register
 s/present/prevent (Chris)
 
 Cc: Art Runyan arthur.j.run...@intel.com
 Signed-off-by: Ben Widawsky b...@bwidawsk.net

Queued for -next, thanks for the patch.
 ---
  drivers/gpu/drm/i915/intel_display.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 7f02444..05c60b1 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3583,10 +3583,13 @@ void hsw_disable_ips(struct intel_crtc *crtc)
  return;
 
  assert_plane_enabled(dev_priv, crtc-plane);
 -if (IS_BROADWELL(crtc-base.dev)) {
 +if (IS_BROADWELL(dev)) {
  mutex_lock(dev_priv-rps.hw_lock);
  WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
 0));
  mutex_unlock(dev_priv-rps.hw_lock);
 +/* wait for pcode to finish disabling IPS, which may take up to 
 42ms */
 +if (wait_for((I915_READ(IPS_CTL)  IPS_ENABLE) == 0, 42))
 +DRM_DEBUG_KMS(Timed out waiting for IPS disable\n);

I've upgraded this to an ERROR to make sure we'll get noticed if it
happens.
-Daniel

  } else {
  I915_WRITE(IPS_CTL, 0);
  POSTING_READ(IPS_CTL);
 --
 1.9.1
 
 ___
 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] drm/i915/bdw: BDW swizzling in done by the memory controller

2014-04-11 Thread Chris Wilson
On Fri, Apr 11, 2014 at 11:09:03AM +0200, Daniel Vetter wrote:
 Afaik the memory controller has always done the swizzling. What this pile
 of bits controls is the _additional_ swizzling done to improve the access
 pattern for 2d data, i.e. everything X and Y tiled. The theory of
 operation is that the additional swizzling improves access patterns when
 walking in Y direction on a surface.
 
 And when we've last looked at this (well Chris) it seemed to indeed have
 improved sampler performace.

And recall this was around SNB time. Jesse keeps claiming that IVB+
doesn't benefit, and I'm inclined to believe him. At some point, we
should test again.
-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 v9 3/6] drm/i915: Add support for DRRS to switch RR

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 02:48:53PM +0530, Vandana Kannan wrote:
 On Apr-10-2014 2:28 PM, Daniel Vetter wrote:
  On Thu, Apr 10, 2014 at 11:43:15AM +0300, Jani Nikula wrote:
 
  Reviewed-by: Jani Nikula jani.nik...@intel.com
 
 
  On Sat, 05 Apr 2014, Vandana Kannan vandana.kan...@intel.com wrote:
  From: Pradeep Bhat pradeep.b...@intel.com
 
  This patch computes and stored 2nd M/N/TU for switching to different
  refresh rate dynamically. PIPECONF_EDP_RR_MODE_SWITCH bit helps toggle
  between alternate refresh rates programmed in 2nd M/N/TU registers.
 
  v2: Daniel's review comments
  Computing M2/N2 in compute_config and storing it in crtc_config
 
  v3: Modified reference to edp_downclock and edp_downclock_avail based on 
  the
  changes made to move them from dev_private to intel_panel.
 
  v4: Modified references to is_drrs_supported based on the changes made to
  rename it to drrs_support.
 
  v5: Jani's review comments
  Removed superfluous return statements. Changed support for Gen 7 and 
  above.
  Corrected indentation. Re-structured the code which finds crtc and 
  connector
  from encoder. Changed some logs to be less verbose.
 
  v6: Modifying i915_drrs to include only intel connector as intel_dp can be
  derived from intel connector when required.
 
  v7: As per internal review comments, acquiring mutex just before accessing
  drrs RR. As per Chris's review comments, added documentation about the use
  of locking in the function.
 
  v8: Incorporated Jani's review comments.
  Removed reference to edp_downclock.
 
  v9: Jani's review comments. Modified comment in set_drrs. Changed index to
  type edp_drrs_refresh_rate_type. Check if PSR is enabled before setting
  registers fo DRRS.
 
  Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
  Signed-off-by: Vandana Kannan vandana.kan...@intel.com
  Cc: Jani Nikula jani.nik...@linux.intel.com
  
  Queued for -next, thanks for the patch. One thing that's missing though is
  the state readout and cross-check support for this new bit of crtc-config
  data. We need to add that before putting this to real use.
  -Daniel
  
 Hi Daniel,
 
 Could you please elaborate on your input above - on the missing code and
 cross-checking for support part?

If you add new state to crtc-config then you need to add the relevan
readout code for that state (see the code in check_crtc_state) and ofc
also add it to intel_pipe_config_compare.

This is a debug feature of our driver to make sure we never lose track of
things and thus far has been extremely helpful in catching issues early.
-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] [PULL] drm-intel-fixes

2014-04-11 Thread Jani Nikula

Hi Dave, here's the first batch of fixes for 3.15.

BR,
Jani.


The following changes since commit c39b06951f1dc2e384650288676c5b7dcc0ec92c:

  DRM: armada: fix corruption while loading cursors (2014-04-08 10:51:03 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-04-11

for you to fetch changes up to 691e6415c891b8b2b082a120b896b443531c4d45:

  drm/i915: Always use kref tracking for all contexts. (2014-04-11 13:29:51 
+0300)


Chris Wilson (1):
  drm/i915: Always use kref tracking for all contexts.

Daniel Vetter (2):
  drm/mm: Don't WARN if drm_mm_reserve_node
  drm/i915: Disable self-refresh for untiled fbs on i915gm

Jani Nikula (2):
  drm/i915: check VBT for supported backlight type
  drm/i915: do not setup backlight if not available according to VBT

 drivers/gpu/drm/drm_mm.c   |2 -
 drivers/gpu/drm/i915/i915_drv.h|9 +-
 drivers/gpu/drm/i915/i915_gem.c|2 +-
 drivers/gpu/drm/i915/i915_gem_context.c|  218 +++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 drivers/gpu/drm/i915/intel_bios.c  |   10 ++
 drivers/gpu/drm/i915/intel_bios.h  |3 +
 drivers/gpu/drm/i915/intel_panel.c |5 +
 drivers/gpu/drm/i915/intel_pm.c|   10 ++
 9 files changed, 122 insertions(+), 139 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center


pgpN0L1kTQl79.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bdw: BDW swizzling in done by the memory controller

2014-04-11 Thread Damien Lespiau
On Fri, Apr 11, 2014 at 11:09:03AM +0200, Daniel Vetter wrote:
 So not yet sold on the story here, at least for gen8/bdw. Apparently
 people haven't screamed about this yet, so it probably works.

Having thought about it a bit more, I don't see how the CPU side would
know about the tiling layout of the surfaces it accesses, so the
remaining options I see:

  - we still need to swizzle the address on the CPU side
  - bit 6 swizzling for X/Y tiling is just gone and the optimal use of
the RAM is left to the memory controller. If that's the case, we
should see things failing soon enough

So, until any failure case, meh. Just one thing to remember is that the
swizzling bits we set are possibly reserved and may be no-ops.

-- 
Damien

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


[Intel-gfx] [PATCH] drm/i915: Adding debug code for dp_m2_n2 in crtc_config

2014-04-11 Thread Vandana Kannan
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.

Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1af1d14..36fc034 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9199,6 +9199,15 @@ static void intel_dump_pipe_config(struct intel_crtc 
*crtc,
  pipe_config-dp_m_n.gmch_m, pipe_config-dp_m_n.gmch_n,
  pipe_config-dp_m_n.link_m, pipe_config-dp_m_n.link_n,
  pipe_config-dp_m_n.tu);
+
+   DRM_DEBUG_KMS(dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: 
%u, tu2: %u\n,
+ pipe_config-has_dp_encoder,
+ pipe_config-dp_m2_n2.gmch_m,
+ pipe_config-dp_m2_n2.gmch_n,
+ pipe_config-dp_m2_n2.link_m,
+ pipe_config-dp_m2_n2.link_n,
+ pipe_config-dp_m2_n2.tu);
+
DRM_DEBUG_KMS(requested mode:\n);
drm_mode_debug_printmodeline(pipe_config-requested_mode);
DRM_DEBUG_KMS(adjusted mode:\n);
@@ -9619,6 +9628,12 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_I(dp_m_n.link_n);
PIPE_CONF_CHECK_I(dp_m_n.tu);
 
+   PIPE_CONF_CHECK_I(dp_m2_n2.gmch_m);
+   PIPE_CONF_CHECK_I(dp_m2_n2.gmch_n);
+   PIPE_CONF_CHECK_I(dp_m2_n2.link_m);
+   PIPE_CONF_CHECK_I(dp_m2_n2.link_n);
+   PIPE_CONF_CHECK_I(dp_m2_n2.tu);
+
PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
-- 
1.9.1

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


Re: [Intel-gfx] [PATCH v9 3/6] drm/i915: Add support for DRRS to switch RR

2014-04-11 Thread Vandana Kannan
On Apr-11-2014 2:56 PM, Daniel Vetter wrote:
 On Fri, Apr 11, 2014 at 02:48:53PM +0530, Vandana Kannan wrote:
 On Apr-10-2014 2:28 PM, Daniel Vetter wrote:
 On Thu, Apr 10, 2014 at 11:43:15AM +0300, Jani Nikula wrote:

 Reviewed-by: Jani Nikula jani.nik...@intel.com


 On Sat, 05 Apr 2014, Vandana Kannan vandana.kan...@intel.com wrote:
 From: Pradeep Bhat pradeep.b...@intel.com

 This patch computes and stored 2nd M/N/TU for switching to different
 refresh rate dynamically. PIPECONF_EDP_RR_MODE_SWITCH bit helps toggle
 between alternate refresh rates programmed in 2nd M/N/TU registers.

 v2: Daniel's review comments
 Computing M2/N2 in compute_config and storing it in crtc_config

 v3: Modified reference to edp_downclock and edp_downclock_avail based on 
 the
 changes made to move them from dev_private to intel_panel.

 v4: Modified references to is_drrs_supported based on the changes made to
 rename it to drrs_support.

 v5: Jani's review comments
 Removed superfluous return statements. Changed support for Gen 7 and 
 above.
 Corrected indentation. Re-structured the code which finds crtc and 
 connector
 from encoder. Changed some logs to be less verbose.

 v6: Modifying i915_drrs to include only intel connector as intel_dp can be
 derived from intel connector when required.

 v7: As per internal review comments, acquiring mutex just before accessing
 drrs RR. As per Chris's review comments, added documentation about the use
 of locking in the function.

 v8: Incorporated Jani's review comments.
 Removed reference to edp_downclock.

 v9: Jani's review comments. Modified comment in set_drrs. Changed index to
 type edp_drrs_refresh_rate_type. Check if PSR is enabled before setting
 registers fo DRRS.

 Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
 Signed-off-by: Vandana Kannan vandana.kan...@intel.com
 Cc: Jani Nikula jani.nik...@linux.intel.com

 Queued for -next, thanks for the patch. One thing that's missing though is
 the state readout and cross-check support for this new bit of crtc-config
 data. We need to add that before putting this to real use.
 -Daniel

 Hi Daniel,

 Could you please elaborate on your input above - on the missing code and
 cross-checking for support part?
 
 If you add new state to crtc-config then you need to add the relevan
 readout code for that state (see the code in check_crtc_state) and ofc
 also add it to intel_pipe_config_compare.
 
 This is a debug feature of our driver to make sure we never lose track of
 things and thus far has been extremely helpful in catching issues early.
 -Daniel
 
I have submitted a patch adding this piece of code. Please have a look..
http://lists.freedesktop.org/archives/intel-gfx/2014-April/043569.html

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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: BDW swizzling in done by the memory controller

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 1:10 PM, Damien Lespiau
damien.lesp...@intel.com wrote:
 On Fri, Apr 11, 2014 at 11:09:03AM +0200, Daniel Vetter wrote:
 So not yet sold on the story here, at least for gen8/bdw. Apparently
 people haven't screamed about this yet, so it probably works.

 Having thought about it a bit more, I don't see how the CPU side would
 know about the tiling layout of the surfaces it accesses, so the
 remaining options I see:

   - we still need to swizzle the address on the CPU side
   - bit 6 swizzling for X/Y tiling is just gone and the optimal use of
 the RAM is left to the memory controller. If that's the case, we
 should see things failing soon enough

 So, until any failure case, meh. Just one thing to remember is that the
 swizzling bits we set are possibly reserved and may be no-ops.

That's very easy to figure out. Set the bit in TILE_CTL and
GAMT_ARBMODE differently and see what happens. blt vs gtt mmap access
would be different. igt has testcases which will catch this.

At least on simulation this blew up rather badly ;-)
-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 v4 3/3] drm/i915: New drm crtc property for varying the size of borders

2014-04-11 Thread Ville Syrjälä
On Fri, Apr 11, 2014 at 08:34:08AM +0530, Akash Goel wrote:
 On Thu, 2014-04-10 at 14:34 +0300, Ville Syrjälä wrote:
  On Thu, Apr 10, 2014 at 01:13:56PM +0530, Akash Goel wrote:
   On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.g...@intel.com wrote:
 From: Akash Goel akash.g...@intel.com
 
 This patch adds a new drm crtc property for varying the size of
 the horizontal  vertical borers of the output/display window.
 This will control the output of Panel fitter.
 
 v2: Added a new check for the invalid border size input
 
 v3: Fixed bugs in output window calculation
 Removed superfluous checks
 
 v4: Added the capability to forecfully enable the Panel fitter.
 The property value is of 64 bits, first 32 bits are used for
 border dimensions. The 33rd bit can be used to forcefully
 enable the panel fitter. This is useful for Panels which
 do not override the User specified Pipe timings.
 
 Testcase: igt/kms_panel_fitter_test
 
 Signed-off-by: Akash Goel akash.g...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h  |   7 ++
  drivers/gpu/drm/i915/intel_display.c |  39 +++-
  drivers/gpu/drm/i915/intel_drv.h |   5 +
  drivers/gpu/drm/i915/intel_panel.c   | 176 
 ---
  4 files changed, 211 insertions(+), 16 deletions(-)
 
  snip
 @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct 
 drm_display_mode *fixed_mode,
   drm_mode_set_crtcinfo(adjusted_mode, 0);
  }
  
 +void
 +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
 + struct intel_crtc_config *pipe_config)
 +{
 + struct drm_display_mode *adjusted_mode;
 + int x, y;
 + u32 pf_horizontal_ratio, pf_vertical_ratio;
 + u32 tot_width, tot_height;
 + u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
 + u32 dst_width, dst_height;
 +
 + adjusted_mode = pipe_config-adjusted_mode;
 +
 + src_width = pipe_config-pipe_src_w;
 + src_height = pipe_config-pipe_src_h;
 +
 + tot_width  = adjusted_mode-hdisplay;
 + tot_height = adjusted_mode-vdisplay;
 +
 + /*
 +  * Having non zero borders will reduce the size of 
 'HACTIVE/VACTIVE'
 +  * region. So (HACTIVE - Left border - Right Border) *
 +  * (VACTIVE  - Top Border  - Bottom border) will effectively be 
 the
 +  * output rectangle on screen
 +  */
 + dst_width = tot_width -
 + (((intel_crtc-border_size  16)  0x) * 
 2);
 + dst_height = tot_height -
 + ((intel_crtc-border_size  0x) * 2);

I'm thinking that we should allow the user to specify each border width
individually rather than just assume left==right and top==bottom.

   
   Sorry I thought that the Top/Bottom  left/Right borders would be
   symmetric only.
  
  I don't see a reason to limit ourselves here.
  
 
 Fine, will extend this property to set each border separately. 
 Can I use the 12 bits for each border value, as that shall be
 sufficient.

Maybe just add separate properties for each border value. We already
have similar properties for TV outputs.

 
   Tried setting the borders on EDP  HDMI panels by manipulating the Pipe
   timings (through the logic used in 'centre_horizontally' 
   'centre_vertically' functions), but it didn't work.
   Is this logic effective for the LVDS panel only ?
  
  Could be. Certainly the border enable bit is there only for LVDS. The
  gmch panel fitter isn't very flexible so it's possible we can't
  actually make it do many of the things the pch pfit can do.
  
 
 Yes the GMCH panel fitter function is not equally capable as the PCH
 counterpart. Here except the LVDS panel, it seems that borders cannot be
 realized on any other panel, just via the manipulation of Pipe timings
 (the way it can be done in PCH one).
 For the same reason the 'Center' Panel fitting mode of scaling mode
 property is not working on VLV  (at least for HDMI/EDP panels).
 
  What happens if we set up the pfit to use manual scaling ratios but
  configure both scaling ratios so that scaled image won't fill the
  active video region in either direction? Does it position the scaled
  image at coordinates 0,0 and simply scan black the rest of the time after
  it's run out of source pixel data? Or does it automagically center the
  image and scan black on both sides? Or does it fail in some way?
  
 
 Already tried that, but in vain. As per the VLV spec, the support for
 Manual scaling ratio mode itself has been de-featured, so it didn't work
 at all. So Auto/LetterBox/PillarBox modes are being supported.

I see.

 
 As a next step tried to manipulate the Pipe timings, so as to produce
 the borders on 4 sides of the panel. 

[Intel-gfx] [Bug] Wrong screen resolution

2014-04-11 Thread Knut Petersen

Description:


Whenever I restart the X server there is a small (20%?) chance that the
KDE splash picture is not displayed as expected (1280x1024).
Instead I see a smaller version of that picture (1024x768?) in the
upper left corner of the monitor, but the correct 1280x1024 video
mode is used by xorg. Absolutely _nothing_ unusual in system/xorg
logs.

After that, everything works fine, with one exception:
mplayer fullscreen mode is broken. The video is displayed
correctly, but the area not used by the video itself is only partially set
to black, the upper right and lower right area contain garbage.
If the splash screen is displayed correctly, mplayer full screen
mode works correctly too.

Most of the time another restart of X corrects the problem.

Any idea how to debug that situation?

Hardware:
===
AOpen i915GMm-hfs, Pentium-M Dothan, Eizo L557 monitor (1280x1024)

Software:
==
openSuSE, KDE, all recent versions of linux kernel and xorg,
last good unknown.

cu,
 Knut

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


Re: [Intel-gfx] [Bug] Wrong screen resolution

2014-04-11 Thread Chris Wilson
On Fri, Apr 11, 2014 at 01:46:40PM +0200, Knut Petersen wrote:
 Description:
 
 
 Whenever I restart the X server there is a small (20%?) chance that the
 KDE splash picture is not displayed as expected (1280x1024).
 Instead I see a smaller version of that picture (1024x768?) in the
 upper left corner of the monitor, but the correct 1280x1024 video
 mode is used by xorg. Absolutely _nothing_ unusual in system/xorg
 logs.
 
 After that, everything works fine, with one exception:
 mplayer fullscreen mode is broken. The video is displayed
 correctly, but the area not used by the video itself is only partially set
 to black, the upper right and lower right area contain garbage.
 If the splash screen is displayed correctly, mplayer full screen
 mode works correctly too.
 
 Most of the time another restart of X corrects the problem.
 
 Any idea how to debug that situation?

What exactly does Xorg.0.log and xrandr say? Have you looked at the
drm.debug=6 dmesg?
-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 i-g-t 3/3] kms_universal_plane: Universal plane testing

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 11:22:39AM +0200, Daniel Vetter wrote:
 On Thu, Apr 10, 2014 at 05:26:25PM -0700, Matt Roper wrote:
  Add a simple test to exercise universal plane support.
  
  Signed-off-by: Matt Roper matthew.d.ro...@intel.com
 
 Looks like a good functional test, but I think we need to add a bit more
 api nastiness still. So no functional tests with CRCs, but just tests to
 make sure the kernel doesn't fall over.
 
 - primary plane set_plane calls vs. legacy setcrtc primary plane updates.
   We'll very likely have mixed userspace (e.g. boot splash vs. display
   manager). E.g. disable primary plane (but keep everything working), then
   setCrtc a new plane framebuffer.
 
 - primary plane vs. other ioctls. Might be easier to extend existing tests
   for this. E.g. doing a pageflip ioctl if the primary plane is off
   (might need to decide what we really want to do and if we decide that it
   should enable the primary plane then we need a CRC based test to make
   sure that the transition is perfect).
 
   Or primary plane changes vs. dpms and suspend/resume. For those
   functional checks based on CRC would be good to make sure we properly
   restore everything.
 
 - Maybe exercise some of the checks in the primary plane helper to make
   sure they work. In the future we'll probably lift those limitations (not
   on current hw afaik though), but then we can adjust those tests to skip
   on these platforms.
 
 - Anything else that was pointed out in review or was tricky while
   developing this stuff.

More ideas! My main concern is interactions with existing features when
the primary plane is disabled and no other plane/cursor enabled, i.e. when
we scan out a solid black.

- solid black vs. dpms for all connector/pipe pairings. The modeset
  sequence is different for different connectors and my gut says not
  enabling the primary plane might cause trouble.

- solid black vs. vblank events. When transition through a no planes
  situation we need to make sure that vblank events keep on working. A
  testcase (maybe in combination of some of the dpms and susped/resume
  case above too) would be good.

I'll leave it to you to somewhat sensibly group all these ideas into real
testcases ;-) For further inspiration maybe look at the corner cases
existing kms_* tests exercise 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


[Intel-gfx] [PATCH] drm/plane_helper: don't disable plane in destroy function

2014-04-11 Thread Daniel Vetter
By the time drm_mode_config_cleanup calls this all the hw state should
be cleaned up already - we even have a WARN right before calling
plane-destroy callbacks asserting that all framebuffers are gone.

So trying to disable things harder is a bit a bug. Caught by Thierry
since it resulted in some mode_config.mutex locking backtraces.

Cc: Thierry Reding tred...@nvidia.com
Cc: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/drm_plane_helper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index e768d35ff22e..a3c9c6e11ee9 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -255,7 +255,6 @@ EXPORT_SYMBOL(drm_primary_helper_disable);
  */
 void drm_primary_helper_destroy(struct drm_plane *plane)
 {
-   plane-funcs-disable_plane(plane);
drm_plane_cleanup(plane);
kfree(plane);
 }
-- 
1.8.5.2

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


Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

2014-04-11 Thread Daniel Vetter
Please don't drop the mailing list when discussing patches.

And nope, conditioning this on gen6+ won't help at all, since I have a
gen6 and gen7 machine here which don't have reliable hdmi live status.
Afaik that is _really_ broken across the board and going with the
fancy invalidation scheme and delayed work items is the way forward.

Cheers, Daniel

On Fri, Apr 11, 2014 at 2:51 PM, Sharma, Shashank
shashank.sha...@intel.com wrote:
 Hi Daniel,

 First of all, sorry for the delay in coming up with the new patch set. I got 
 stuck with some commercial projects :) .

 There were few review comments what you gave for the previous optimization 
 patch, those were:
 1.  This might break the old HWs. Few of them might not even be HPD capable, 
 so its required to handle them.
 2.  Do not rely on the live_status check, as that HW is not yet proven.
 And you recommended to have a WQ, which will invalidate the cached HDMI EDID 
 after a timeout.

 I have written another patch, which is addressing both of the above comments, 
 but doesn't use a WQ.

 Before sending this to ML, I wanted to have an opinion from you, if this 
 qualifies the design you were expecting.
 I will be really glad, if you can spare some time, and just have a top level 
 view on the patch, and give your opinion.

 If you think that this is still not the way, and making a WQ is a better 
 approach, I would gladly create a patch which applies that.
 Or if you will prefer to have this reviewed in the mailing list only, please 
 accept my apologies for direct contact, and just let me know so. I will 
 publish this in ML.

 The patch is (It's also attached):

 From e7ec4e4827615c0ff3a4ddb020df35fdf1b0b82f Mon Sep 17 00:00:00 2001
 From: Shashank Sharma shashank.sha...@intel.com
 Date: Fri, 11 Apr 2014 18:02:50 +0530
 Subject: [PATCH] drm/i915: Cache HDMI EDID for future reference

 This patch contains following changes:
 1.Cache HDMI EDID and optimize detect() calls, which are
   frequently called from userspace, causing un-necessary
   EDID reads.
 2.This cached EDID will be used for detection of the status of
   HDMI connector, for Non HPD detect() calls. HPD calls will update
   cached EDID.
 3.This optimization is kept for new HW's (Gen6 and +), so that It
   will not break old HWs who may not be even HPD capable.

 Signed-off-by: Shashank Sharma shashank.sha...@intel.com
 ---
  drivers/gpu/drm/i915/intel_drv.h  |1 +
  drivers/gpu/drm/i915/intel_hdmi.c |   28 ++--
  2 files changed, 27 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 42762b7..b7911df 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -478,6 +478,7 @@ struct intel_hdmi {
 const void *frame, ssize_t len);
 void (*set_infoframes)(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode);
 +   struct edid *edid;
  };

  #define DP_MAX_DOWNSTREAM_PORTS0x10
 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index b0413e1..33550ca 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -938,7 +938,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
 force)
 hdmi_to_dig_port(intel_hdmi);
 struct intel_encoder *intel_encoder = intel_dig_port-base;
 struct drm_i915_private *dev_priv = dev-dev_private;
 -   struct edid *edid;
 +   struct edid *edid = NULL;
 enum intel_display_power_domain power_domain;
 enum drm_connector_status status = connector_status_disconnected;

 @@ -948,6 +948,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
 force)
 power_domain = intel_display_port_power_domain(intel_encoder);
 intel_display_power_get(dev_priv, power_domain);

 +   /*
 +   * Support EDID caching only for new architectures
 +   * Let old architectures probe and force read EDID
 +   */
 +   if (INTEL_INFO(dev)-gen = 6) {
 +   if (force  intel_hdmi-edid 
 +   (connector-status != connector_status_unknown)) {
 +   /* Optimize userspace query, read EDID only
 +   in case of a real hot plug */
 +   DRM_DEBUG_KMS(HDMI %s, intel_hdmi-edid ?
 +   Connected : Disconnected);
 +   return connector-status;
 +   }
 +   }
 +
 intel_hdmi-has_hdmi_sink = false;
 intel_hdmi-has_audio = false;
 intel_hdmi-rgb_quant_range_selectable = false;
 @@ -964,10 +979,19 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
 force)
 intel_hdmi-has_audio = 
 drm_detect_monitor_audio(edid);
 intel_hdmi-rgb_quant_range_selectable =
 

[Intel-gfx] [PATCH] tests/gem_error_capture: Initial testcase for error state capture/dump

2014-04-11 Thread oscar . mateo
From: Oscar Mateo oscar.ma...@intel.com

Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 tests/.gitignore  |   1 +
 tests/Makefile.sources|   1 +
 tests/gem_error_capture.c | 230 ++
 3 files changed, 232 insertions(+)
 create mode 100644 tests/gem_error_capture.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..945574c 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -27,6 +27,7 @@ gem_ctx_create
 gem_ctx_exec
 gem_double_irq_loop
 gem_dummy_reloc_loop
+gem_error_capture
 gem_evict_alignment
 gem_evict_everything
 gem_exec_bad_domains
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index bf02a48..612beb6 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -24,6 +24,7 @@ TESTS_progs_M = \
gem_ctx_bad_exec \
gem_ctx_exec \
gem_dummy_reloc_loop \
+   gem_error_capture \
gem_evict_alignment \
gem_evict_everything \
gem_exec_bad_domains \
diff --git a/tests/gem_error_capture.c b/tests/gem_error_capture.c
new file mode 100644
index 000..bbf0f5d
--- /dev/null
+++ b/tests/gem_error_capture.c
@@ -0,0 +1,230 @@
+/*
+ * Copyright © 2014 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.
+ *
+ * Authors:
+ *Oscar Mateo oscar.ma...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Check whether basic error state capture/dump mechanism is 
correctly
+ * working for all rings.
+ */
+
+#include unistd.h
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include drm.h
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_io.h
+#include igt_debugfs.h
+
+#define MAGIC_NUMBER 0x10001
+uint32_t batch[4] = {MI_NOOP, MI_BATCH_BUFFER_END, MAGIC_NUMBER, MAGIC_NUMBER};
+
+static void stop_rings(void)
+{
+   int fd;
+   static const char buf[] = 0xf;
+
+   fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+
+   close(fd);
+}
+
+static bool rings_stopped(void)
+{
+   int fd;
+   static char buf[128];
+   unsigned long long val;
+
+   fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(read(fd, buf, sizeof(buf))  0);
+   close(fd);
+
+   sscanf(buf, %llx, val);
+
+   return (bool)val;
+}
+
+static void clear_error_state(void)
+{
+   int fd;
+   static const char buf[] = ;
+
+   fd = igt_debugfs_open(i915_error_state, O_WRONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+   close(fd);
+}
+
+static void check_bb_contents(char **line, size_t *line_size,
+   FILE *file, int pos, uint32_t expected_value)
+{
+   char expected_line[32];
+
+   igt_assert(getline(line, line_size, file)  0);
+
+   snprintf(expected_line, sizeof(expected_line), %08x :  %08x,
+   4*pos, expected_value);
+
+   igt_assert(strstr(*line, expected_line));
+}
+
+static void check_error_state(const char *expected_ring_name,
+   uint64_t expected_offset)
+{
+   FILE *file;
+   int debug_fd;
+   char *line = NULL;
+   size_t line_size;
+   char *dashes = NULL;
+   char *ring_name = NULL;
+   uint32_t gtt_offset = 0;
+   bool matched;
+   int i;
+
+   debug_fd = igt_debugfs_open(i915_error_state, O_RDONLY);
+   igt_assert(debug_fd = 0);
+   file = fdopen(debug_fd, r);
+
+   while (getline(line, line_size, file)  0) {
+   dashes = strstr(line, ---);
+   if (dashes) {
+   ring_name = realloc(ring_name, dashes - line);
+   strncpy(ring_name, line, dashes - line);
+  

Re: [Intel-gfx] [PATCH] tests/gem_error_capture: Initial testcase for error state capture/dump

2014-04-11 Thread Mateo Lozano, Oscar
Daniel, is this what you had in mind?

-- Oscar

P.S. I just re-read the Jira task and realized that I am missing the check 
that the ring objects contains a reloc with MI_BB_START for your presumed batch 
object's address. I´ll add this and resubmit.

 -Original Message-
 From: Mateo Lozano, Oscar
 Sent: Friday, April 11, 2014 2:00 PM
 To: intel-gfx@lists.freedesktop.org
 Cc: Mateo Lozano, Oscar
 Subject: [PATCH] tests/gem_error_capture: Initial testcase for error state
 capture/dump
 
 From: Oscar Mateo oscar.ma...@intel.com
 
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 ---
  tests/.gitignore  |   1 +
  tests/Makefile.sources|   1 +
  tests/gem_error_capture.c | 230
 ++
  3 files changed, 232 insertions(+)
  create mode 100644 tests/gem_error_capture.c
 
 diff --git a/tests/.gitignore b/tests/.gitignore index 146bab0..945574c 100644
 --- a/tests/.gitignore
 +++ b/tests/.gitignore
 @@ -27,6 +27,7 @@ gem_ctx_create
  gem_ctx_exec
  gem_double_irq_loop
  gem_dummy_reloc_loop
 +gem_error_capture
  gem_evict_alignment
  gem_evict_everything
  gem_exec_bad_domains
 diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
 bf02a48..612beb6 100644
 --- a/tests/Makefile.sources
 +++ b/tests/Makefile.sources
 @@ -24,6 +24,7 @@ TESTS_progs_M = \
   gem_ctx_bad_exec \
   gem_ctx_exec \
   gem_dummy_reloc_loop \
 + gem_error_capture \
   gem_evict_alignment \
   gem_evict_everything \
   gem_exec_bad_domains \
 diff --git a/tests/gem_error_capture.c b/tests/gem_error_capture.c new file
 mode 100644 index 000..bbf0f5d
 --- /dev/null
 +++ b/tests/gem_error_capture.c
 @@ -0,0 +1,230 @@
 +/*
 + * Copyright © 2014 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.
 + *
 + * Authors:
 + *Oscar Mateo oscar.ma...@intel.com
 + *
 + */
 +
 +/*
 + * Testcase: Check whether basic error state capture/dump mechanism is
 +correctly
 + * working for all rings.
 + */
 +
 +#include unistd.h
 +#include stdlib.h
 +#include stdint.h
 +#include stdio.h
 +#include string.h
 +#include fcntl.h
 +#include inttypes.h
 +#include errno.h
 +#include sys/stat.h
 +#include sys/ioctl.h
 +#include drm.h
 +#include ioctl_wrappers.h
 +#include drmtest.h
 +#include intel_io.h
 +#include igt_debugfs.h
 +
 +#define MAGIC_NUMBER 0x10001
 +uint32_t batch[4] = {MI_NOOP, MI_BATCH_BUFFER_END, MAGIC_NUMBER,
 +MAGIC_NUMBER};
 +
 +static void stop_rings(void)
 +{
 + int fd;
 + static const char buf[] = 0xf;
 +
 + fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
 + igt_assert(fd = 0);
 +
 + igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
 +
 + close(fd);
 +}
 +
 +static bool rings_stopped(void)
 +{
 + int fd;
 + static char buf[128];
 + unsigned long long val;
 +
 + fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
 + igt_assert(fd = 0);
 +
 + igt_assert(read(fd, buf, sizeof(buf))  0);
 + close(fd);
 +
 + sscanf(buf, %llx, val);
 +
 + return (bool)val;
 +}
 +
 +static void clear_error_state(void)
 +{
 + int fd;
 + static const char buf[] = ;
 +
 + fd = igt_debugfs_open(i915_error_state, O_WRONLY);
 + igt_assert(fd = 0);
 +
 + igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
 + close(fd);
 +}
 +
 +static void check_bb_contents(char **line, size_t *line_size,
 + FILE *file, int pos, uint32_t expected_value) {
 + char expected_line[32];
 +
 + igt_assert(getline(line, line_size, file)  0);
 +
 + snprintf(expected_line, sizeof(expected_line), %08x :  %08x,
 + 4*pos, expected_value);
 +
 + igt_assert(strstr(*line, expected_line)); }
 +
 +static void check_error_state(const char *expected_ring_name,
 + uint64_t expected_offset)
 +{
 + FILE *file;
 + int 

Re: [Intel-gfx] [PATCH] drm/i915: get power domain in case the BIOS enabled eDP VDD

2014-04-11 Thread Daniel Vetter
On Wed, Apr 09, 2014 at 06:47:33PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 If I unplug the eDP monitor, the BIOS of my machine will enable the
 VDD bit, then when the driver loads it will think VDD is enabled. It
 will detect that the eDP is not enabled and return false from
 intel_edp_init_connector. This will trigger a call to
 edp_panel_vdd_off_sync(), which trigger a WARN saying that the
 refcount of the power domain is less than zero.
 
 The problem happens because the driver gets a refcount whenever it
 enables the VDD bit, and puts the refcount whenever it disables the
 VDD bit. But on this case, the BIOS enabled VDD, so all we do is to
 call put() without calling get() first, so the code added is there to
 make sure we always have the get() in case the BIOS enabled the bit.
 
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com

Probably also

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76987

Paulo can you please polish the patch as requested by Chris and resubmit?
Please also repoke the bug.

Thanks, Daniel

 ---
  drivers/gpu/drm/i915/intel_dp.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index e48d47c..a432904 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3638,7 +3638,8 @@ static bool intel_edp_init_connector(struct intel_dp 
 *intel_dp,
  {
   struct drm_connector *connector = intel_connector-base;
   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 - struct drm_device *dev = intel_dig_port-base.base.dev;
 + struct intel_encoder *intel_encoder = intel_dig_port-base;
 + struct drm_device *dev = intel_encoder-base.dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_display_mode *fixed_mode = NULL;
   bool has_dpcd;
 @@ -3648,6 +3649,14 @@ static bool intel_edp_init_connector(struct intel_dp 
 *intel_dp,
   if (!is_edp(intel_dp))
   return true;
  
 + /* The VDD bit needs a power domain reference, so if the bit is already
 +  * enabled when we boot, grab this reference. */
 + if (edp_have_panel_vdd(intel_dp)) {
 + enum intel_display_power_domain power_domain;
 + power_domain = intel_display_port_power_domain(intel_encoder);
 + intel_display_power_get(dev_priv, power_domain);
 + }
 +
   /* Cache DPCD and EDID for edp. */
   intel_edp_panel_vdd_on(intel_dp);
   has_dpcd = intel_dp_get_dpcd(intel_dp);
 -- 
 1.9.0
 
 ___
 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 0/2] Optimization on intel HDMI detect and get_modes

2014-04-11 Thread Sharma, Shashank
Thanks for the comments, 
Actually, we are not using live_status at all. 

The check for  gen6 is only for EDID caching. So if the HW is = gen6 
cache_edid.  
Else do not cache EDID, so that we will not block any of the old HW, which 
might not be HPD capable.

Does it sound ok now :) ?  

Regards
Shashank 
-Original Message-
From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
Daniel Vetter
Sent: Friday, April 11, 2014 6:28 PM
To: Sharma, Shashank
Cc: C, Ramalingam; Wang, Quanxian; intel-gfx
Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and 
get_modes

Please don't drop the mailing list when discussing patches.

And nope, conditioning this on gen6+ won't help at all, since I have a
gen6 and gen7 machine here which don't have reliable hdmi live status.
Afaik that is _really_ broken across the board and going with the fancy 
invalidation scheme and delayed work items is the way forward.

Cheers, Daniel

On Fri, Apr 11, 2014 at 2:51 PM, Sharma, Shashank shashank.sha...@intel.com 
wrote:
 Hi Daniel,

 First of all, sorry for the delay in coming up with the new patch set. I got 
 stuck with some commercial projects :) .

 There were few review comments what you gave for the previous optimization 
 patch, those were:
 1.  This might break the old HWs. Few of them might not even be HPD capable, 
 so its required to handle them.
 2.  Do not rely on the live_status check, as that HW is not yet proven.
 And you recommended to have a WQ, which will invalidate the cached HDMI EDID 
 after a timeout.

 I have written another patch, which is addressing both of the above comments, 
 but doesn't use a WQ.

 Before sending this to ML, I wanted to have an opinion from you, if this 
 qualifies the design you were expecting.
 I will be really glad, if you can spare some time, and just have a top level 
 view on the patch, and give your opinion.

 If you think that this is still not the way, and making a WQ is a better 
 approach, I would gladly create a patch which applies that.
 Or if you will prefer to have this reviewed in the mailing list only, please 
 accept my apologies for direct contact, and just let me know so. I will 
 publish this in ML.

 The patch is (It's also attached):

 From e7ec4e4827615c0ff3a4ddb020df35fdf1b0b82f Mon Sep 17 00:00:00 2001
 From: Shashank Sharma shashank.sha...@intel.com
 Date: Fri, 11 Apr 2014 18:02:50 +0530
 Subject: [PATCH] drm/i915: Cache HDMI EDID for future reference

 This patch contains following changes:
 1.Cache HDMI EDID and optimize detect() calls, which are
   frequently called from userspace, causing un-necessary
   EDID reads.
 2.This cached EDID will be used for detection of the status of
   HDMI connector, for Non HPD detect() calls. HPD calls will update
   cached EDID.
 3.This optimization is kept for new HW's (Gen6 and +), so that It
   will not break old HWs who may not be even HPD capable.

 Signed-off-by: Shashank Sharma shashank.sha...@intel.com
 ---
  drivers/gpu/drm/i915/intel_drv.h  |1 +
  drivers/gpu/drm/i915/intel_hdmi.c |   28 ++--
  2 files changed, 27 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 42762b7..b7911df 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -478,6 +478,7 @@ struct intel_hdmi {
 const void *frame, ssize_t len);
 void (*set_infoframes)(struct drm_encoder *encoder,
struct drm_display_mode 
 *adjusted_mode);
 +   struct edid *edid;
  };

  #define DP_MAX_DOWNSTREAM_PORTS0x10
 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index b0413e1..33550ca 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -938,7 +938,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
 force)
 hdmi_to_dig_port(intel_hdmi);
 struct intel_encoder *intel_encoder = intel_dig_port-base;
 struct drm_i915_private *dev_priv = dev-dev_private;
 -   struct edid *edid;
 +   struct edid *edid = NULL;
 enum intel_display_power_domain power_domain;
 enum drm_connector_status status = 
 connector_status_disconnected;

 @@ -948,6 +948,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
 force)
 power_domain = intel_display_port_power_domain(intel_encoder);
 intel_display_power_get(dev_priv, power_domain);

 +   /*
 +   * Support EDID caching only for new architectures
 +   * Let old architectures probe and force read EDID
 +   */
 +   if (INTEL_INFO(dev)-gen = 6) {
 +   if (force  intel_hdmi-edid 
 +   (connector-status != connector_status_unknown)) {
 +   /* Optimize userspace query, read EDID only
 +   in case of a real hot plug */
 + 

Re: [Intel-gfx] [PATCH] drm/plane_helper: don't disable plane in destroy function

2014-04-11 Thread Thierry Reding
On Fri, Apr 11, 2014 at 02:12:10PM +0200, Daniel Vetter wrote:
 By the time drm_mode_config_cleanup calls this all the hw state should
 be cleaned up already - we even have a WARN right before calling
 plane-destroy callbacks asserting that all framebuffers are gone.
 
 So trying to disable things harder is a bit a bug. Caught by Thierry
 since it resulted in some mode_config.mutex locking backtraces.
 
 Cc: Thierry Reding tred...@nvidia.com
 Cc: Matt Roper matthew.d.ro...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/drm_plane_helper.c | 1 -
  1 file changed, 1 deletion(-)

Reviewed-by: Thierry Reding tred...@nvidia.com
Tested-by: Thierry Reding tred...@nvidia.com


pgpWrUUpKUZRg.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Don't complain about stolen conflicts on gen3

2014-04-11 Thread Daniel Vetter
Apparently stuff works that way on those machines.

I agree with Chris' concern that this is a bit risky but imo worth a
shot in -next just for fun. Afaics all these machines have the pci
resources allocated like that by the BIOS, so I suspect that it's all
ok.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76983
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71031
Tested-by: lu hua huax...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 62ef55ba061c..99d147af173a 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -93,7 +93,11 @@ static unsigned long i915_stolen_to_physical(struct 
drm_device *dev)
r = devm_request_mem_region(dev-dev, base + 1,
dev_priv-gtt.stolen_size - 1,
Graphics Stolen Memory);
-   if (r == NULL) {
+   /*
+* GEN3 firmware likes to smash pci bridges into the stolen
+* range. Apparently this works.
+*/
+   if (r == NULL  !IS_GEN3(dev)) {
DRM_ERROR(conflict detected with stolen region: 
[0x%08x - 0x%08x]\n,
  base, base + 
(uint32_t)dev_priv-gtt.stolen_size);
base = 0;
-- 
1.8.5.2

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


Re: [Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v2)

2014-04-11 Thread Matt Roper
On Fri, Apr 11, 2014 at 11:34:36AM +0200, Daniel Vetter wrote:
 On Thu, Apr 10, 2014 at 05:24:36PM -0700, Matt Roper wrote:
...
  +   /* setplane API takes shifted source rectangle values; unshift them */
  +   src_x = 16;
  +   src_y = 16;
  +   src_w = 16;
  +   src_h = 16;
  +
  +   /*
  +* Current hardware can't reposition the primary plane or scale it
  +* (although this could change in the future).
  +*/
  +   drm_rect_intersect(dest, clip);
  +   if (dest.x1 != 0 || dest.y1 != 0 ||
  +   dest.x2 != crtc-mode.hdisplay || dest.y2 != crtc-mode.vdisplay) {
  +   DRM_DEBUG_KMS(Primary plane must cover entire CRTC\n);
  +   return -EINVAL;
  +   }
  +
  +   if (crtc_w != src_w || crtc_h != src_h) {
  +   DRM_DEBUG_KMS(Can't scale primary plane\n);
  +   return -EINVAL;
  +   }
 
 Subpixel check seems to be missing. And can't we extract all these checks
 both here and from the primary plane helper? I guess there'll be other hw
 which doesn't have scaling primary planes, but which wants to allow
 primary plane enable/disable.

I was a bit unsure about this.  At first I thought I needed to check the
subpixel part, but the DocBook reference indicates

Devices that don't support subpixel plane coordinates can ignore
the fractional part.

which sounds to me like we're supposed to just silently ignore the
subpixel bits on i915 and other devices that don't support it.  Which
would probably also mean that I should remove the (subpixel bits == 0)
test from the primary helper...


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] tests/kms_flip_tiling: Fixes

2014-04-11 Thread Daniel Vetter
- Wrap up testcase correctly into the magic code block.
- Put local variables out of the longjmp danger zone.

Cc: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 tests/kms_flip_tiling.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index e70609d52e78..ca20ad96bc35 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -119,11 +119,10 @@ test_flip_changes_tiling(data_t *data, igt_output_t 
*output)
 }
 
 static data_t data;
+igt_output_t *output;
 
 igt_main
 {
-   igt_output_t *output;
-
igt_skip_on_simulation();
 
igt_fixture {
@@ -135,10 +134,10 @@ igt_main
igt_display_init(data.display, data.drm_fd);
}
 
-   igt_subtest_f(flip-changes-tiling);
-
-   for_each_connected_output(data.display, output)
-   test_flip_changes_tiling(data, output);
+   igt_subtest_f(flip-changes-tiling) {
+   for_each_connected_output(data.display, output)
+   test_flip_changes_tiling(data, output);
+   }
 
igt_fixture {
igt_display_fini(data.display);
-- 
1.8.5.2

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


Re: [Intel-gfx] [PATCH] drm/plane_helper: don't disable plane in destroy function

2014-04-11 Thread Matt Roper
On Fri, Apr 11, 2014 at 02:12:10PM +0200, Daniel Vetter wrote:
 By the time drm_mode_config_cleanup calls this all the hw state should
 be cleaned up already - we even have a WARN right before calling
 plane-destroy callbacks asserting that all framebuffers are gone.
 
 So trying to disable things harder is a bit a bug. Caught by Thierry
 since it resulted in some mode_config.mutex locking backtraces.
 
 Cc: Thierry Reding tred...@nvidia.com
 Cc: Matt Roper matthew.d.ro...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Reviewed-by: Matt Roper matthew.d.ro...@intel.com

 ---
  drivers/gpu/drm/drm_plane_helper.c | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/drm_plane_helper.c 
 b/drivers/gpu/drm/drm_plane_helper.c
 index e768d35ff22e..a3c9c6e11ee9 100644
 --- a/drivers/gpu/drm/drm_plane_helper.c
 +++ b/drivers/gpu/drm/drm_plane_helper.c
 @@ -255,7 +255,6 @@ EXPORT_SYMBOL(drm_primary_helper_disable);
   */
  void drm_primary_helper_destroy(struct drm_plane *plane)
  {
 - plane-funcs-disable_plane(plane);
   drm_plane_cleanup(plane);
   kfree(plane);
  }
 -- 
 1.8.5.2
 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 01:23:43PM +, Sharma, Shashank wrote:
 Thanks for the comments, 
 Actually, we are not using live_status at all. 
 
 The check for  gen6 is only for EDID caching. So if the HW is = gen6 
 cache_edid.  
 Else do not cache EDID, so that we will not block any of the old HW, which 
 might not be HPD capable.

Oh, I've thought that this is incremental on top of something you already
have.
 
 Does it sound ok now :) ?  

No. HPD is _NOT_ I repeat _NOT_ reliably. Not even on gen6+. live status
simply reflects the hpd pin, if either doesn't work, then neither does the
other one.

Nacked-with-prejudice-by: Daniel Vetter daniel.vet...@ffwll.ch

Cheers, Daniel

 
 Regards
 Shashank 
 -Original Message-
 From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of 
 Daniel Vetter
 Sent: Friday, April 11, 2014 6:28 PM
 To: Sharma, Shashank
 Cc: C, Ramalingam; Wang, Quanxian; intel-gfx
 Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and 
 get_modes
 
 Please don't drop the mailing list when discussing patches.
 
 And nope, conditioning this on gen6+ won't help at all, since I have a
 gen6 and gen7 machine here which don't have reliable hdmi live status.
 Afaik that is _really_ broken across the board and going with the fancy 
 invalidation scheme and delayed work items is the way forward.
 
 Cheers, Daniel
 
 On Fri, Apr 11, 2014 at 2:51 PM, Sharma, Shashank shashank.sha...@intel.com 
 wrote:
  Hi Daniel,
 
  First of all, sorry for the delay in coming up with the new patch set. I 
  got stuck with some commercial projects :) .
 
  There were few review comments what you gave for the previous optimization 
  patch, those were:
  1.  This might break the old HWs. Few of them might not even be HPD 
  capable, so its required to handle them.
  2.  Do not rely on the live_status check, as that HW is not yet proven.
  And you recommended to have a WQ, which will invalidate the cached HDMI 
  EDID after a timeout.
 
  I have written another patch, which is addressing both of the above 
  comments, but doesn't use a WQ.
 
  Before sending this to ML, I wanted to have an opinion from you, if this 
  qualifies the design you were expecting.
  I will be really glad, if you can spare some time, and just have a top 
  level view on the patch, and give your opinion.
 
  If you think that this is still not the way, and making a WQ is a better 
  approach, I would gladly create a patch which applies that.
  Or if you will prefer to have this reviewed in the mailing list only, 
  please accept my apologies for direct contact, and just let me know so. I 
  will publish this in ML.
 
  The patch is (It's also attached):
 
  From e7ec4e4827615c0ff3a4ddb020df35fdf1b0b82f Mon Sep 17 00:00:00 2001
  From: Shashank Sharma shashank.sha...@intel.com
  Date: Fri, 11 Apr 2014 18:02:50 +0530
  Subject: [PATCH] drm/i915: Cache HDMI EDID for future reference
 
  This patch contains following changes:
  1.Cache HDMI EDID and optimize detect() calls, which are
frequently called from userspace, causing un-necessary
EDID reads.
  2.This cached EDID will be used for detection of the status of
HDMI connector, for Non HPD detect() calls. HPD calls will update
cached EDID.
  3.This optimization is kept for new HW's (Gen6 and +), so that It
will not break old HWs who may not be even HPD capable.
 
  Signed-off-by: Shashank Sharma shashank.sha...@intel.com
  ---
   drivers/gpu/drm/i915/intel_drv.h  |1 +
   drivers/gpu/drm/i915/intel_hdmi.c |   28 ++--
   2 files changed, 27 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_drv.h 
  b/drivers/gpu/drm/i915/intel_drv.h
  index 42762b7..b7911df 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -478,6 +478,7 @@ struct intel_hdmi {
  const void *frame, ssize_t len);
  void (*set_infoframes)(struct drm_encoder *encoder,
 struct drm_display_mode 
  *adjusted_mode);
  +   struct edid *edid;
   };
 
   #define DP_MAX_DOWNSTREAM_PORTS0x10
  diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
  b/drivers/gpu/drm/i915/intel_hdmi.c
  index b0413e1..33550ca 100644
  --- a/drivers/gpu/drm/i915/intel_hdmi.c
  +++ b/drivers/gpu/drm/i915/intel_hdmi.c
  @@ -938,7 +938,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
  force)
  hdmi_to_dig_port(intel_hdmi);
  struct intel_encoder *intel_encoder = intel_dig_port-base;
  struct drm_i915_private *dev_priv = dev-dev_private;
  -   struct edid *edid;
  +   struct edid *edid = NULL;
  enum intel_display_power_domain power_domain;
  enum drm_connector_status status = 
  connector_status_disconnected;
 
  @@ -948,6 +948,21 @@ intel_hdmi_detect(struct drm_connector *connector, 
  bool force)
  power_domain = 

Re: [Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v2)

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 07:17:41AM -0700, Matt Roper wrote:
 On Fri, Apr 11, 2014 at 11:34:36AM +0200, Daniel Vetter wrote:
  On Thu, Apr 10, 2014 at 05:24:36PM -0700, Matt Roper wrote:
 ...
   + /* setplane API takes shifted source rectangle values; unshift them */
   + src_x = 16;
   + src_y = 16;
   + src_w = 16;
   + src_h = 16;
   +
   + /*
   +  * Current hardware can't reposition the primary plane or scale it
   +  * (although this could change in the future).
   +  */
   + drm_rect_intersect(dest, clip);
   + if (dest.x1 != 0 || dest.y1 != 0 ||
   + dest.x2 != crtc-mode.hdisplay || dest.y2 != crtc-mode.vdisplay) {
   + DRM_DEBUG_KMS(Primary plane must cover entire CRTC\n);
   + return -EINVAL;
   + }
   +
   + if (crtc_w != src_w || crtc_h != src_h) {
   + DRM_DEBUG_KMS(Can't scale primary plane\n);
   + return -EINVAL;
   + }
  
  Subpixel check seems to be missing. And can't we extract all these checks
  both here and from the primary plane helper? I guess there'll be other hw
  which doesn't have scaling primary planes, but which wants to allow
  primary plane enable/disable.
 
 I was a bit unsure about this.  At first I thought I needed to check the
 subpixel part, but the DocBook reference indicates
 
 Devices that don't support subpixel plane coordinates can ignore
 the fractional part.
 
 which sounds to me like we're supposed to just silently ignore the
 subpixel bits on i915 and other devices that don't support it.  Which
 would probably also mean that I should remove the (subpixel bits == 0)
 test from the primary helper...

Hm ... yeah I guess you're right. For now it probably won't matter too
much.
-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] tests/gem_error_capture: Initial testcase for error state capture/dump

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 01:09:22PM +, Mateo Lozano, Oscar wrote:
 Daniel, is this what you had in mind?
 
 -- Oscar
 
 P.S. I just re-read the Jira task and realized that I am missing the
 check that the ring objects contains a reloc with MI_BB_START for your
 presumed batch object's address. I´ll add this and resubmit.

Yeah this looks pretty cool. btw when you resend can you simply sign up
someone local to your team for the review? Bit easier that way.

I've checked your usage of igt_* infrastructure and that looks good.

Thanks, Daniel

 
  -Original Message-
  From: Mateo Lozano, Oscar
  Sent: Friday, April 11, 2014 2:00 PM
  To: intel-gfx@lists.freedesktop.org
  Cc: Mateo Lozano, Oscar
  Subject: [PATCH] tests/gem_error_capture: Initial testcase for error state
  capture/dump
  
  From: Oscar Mateo oscar.ma...@intel.com
  
  Signed-off-by: Oscar Mateo oscar.ma...@intel.com
  ---
   tests/.gitignore  |   1 +
   tests/Makefile.sources|   1 +
   tests/gem_error_capture.c | 230
  ++
   3 files changed, 232 insertions(+)
   create mode 100644 tests/gem_error_capture.c
  
  diff --git a/tests/.gitignore b/tests/.gitignore index 146bab0..945574c 
  100644
  --- a/tests/.gitignore
  +++ b/tests/.gitignore
  @@ -27,6 +27,7 @@ gem_ctx_create
   gem_ctx_exec
   gem_double_irq_loop
   gem_dummy_reloc_loop
  +gem_error_capture
   gem_evict_alignment
   gem_evict_everything
   gem_exec_bad_domains
  diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
  bf02a48..612beb6 100644
  --- a/tests/Makefile.sources
  +++ b/tests/Makefile.sources
  @@ -24,6 +24,7 @@ TESTS_progs_M = \
  gem_ctx_bad_exec \
  gem_ctx_exec \
  gem_dummy_reloc_loop \
  +   gem_error_capture \
  gem_evict_alignment \
  gem_evict_everything \
  gem_exec_bad_domains \
  diff --git a/tests/gem_error_capture.c b/tests/gem_error_capture.c new file
  mode 100644 index 000..bbf0f5d
  --- /dev/null
  +++ b/tests/gem_error_capture.c
  @@ -0,0 +1,230 @@
  +/*
  + * Copyright © 2014 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.
  + *
  + * Authors:
  + *Oscar Mateo oscar.ma...@intel.com
  + *
  + */
  +
  +/*
  + * Testcase: Check whether basic error state capture/dump mechanism is
  +correctly
  + * working for all rings.
  + */
  +
  +#include unistd.h
  +#include stdlib.h
  +#include stdint.h
  +#include stdio.h
  +#include string.h
  +#include fcntl.h
  +#include inttypes.h
  +#include errno.h
  +#include sys/stat.h
  +#include sys/ioctl.h
  +#include drm.h
  +#include ioctl_wrappers.h
  +#include drmtest.h
  +#include intel_io.h
  +#include igt_debugfs.h
  +
  +#define MAGIC_NUMBER 0x10001
  +uint32_t batch[4] = {MI_NOOP, MI_BATCH_BUFFER_END, MAGIC_NUMBER,
  +MAGIC_NUMBER};
  +
  +static void stop_rings(void)
  +{
  +   int fd;
  +   static const char buf[] = 0xf;
  +
  +   fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
  +   igt_assert(fd = 0);
  +
  +   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
  +
  +   close(fd);
  +}
  +
  +static bool rings_stopped(void)
  +{
  +   int fd;
  +   static char buf[128];
  +   unsigned long long val;
  +
  +   fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
  +   igt_assert(fd = 0);
  +
  +   igt_assert(read(fd, buf, sizeof(buf))  0);
  +   close(fd);
  +
  +   sscanf(buf, %llx, val);
  +
  +   return (bool)val;
  +}
  +
  +static void clear_error_state(void)
  +{
  +   int fd;
  +   static const char buf[] = ;
  +
  +   fd = igt_debugfs_open(i915_error_state, O_WRONLY);
  +   igt_assert(fd = 0);
  +
  +   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
  +   close(fd);
  +}
  +
  +static void check_bb_contents(char **line, size_t *line_size,
  +   FILE *file, int pos, uint32_t expected_value) {
  +   char 

Re: [Intel-gfx] [PATCH] tests/kms_flip_tiling: Fixes

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 04:17:48PM +0200, Daniel Vetter wrote:
 - Wrap up testcase correctly into the magic code block.
 - Put local variables out of the longjmp danger zone.
 
 Cc: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

Ander, quick one for process: Please submit igt patches to the mailing
list so that they don't get lost in bugzilla.

And thanks a lot for the testcase.
-Daniel

 ---
  tests/kms_flip_tiling.c | 11 +--
  1 file changed, 5 insertions(+), 6 deletions(-)
 
 diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
 index e70609d52e78..ca20ad96bc35 100644
 --- a/tests/kms_flip_tiling.c
 +++ b/tests/kms_flip_tiling.c
 @@ -119,11 +119,10 @@ test_flip_changes_tiling(data_t *data, igt_output_t 
 *output)
  }
  
  static data_t data;
 +igt_output_t *output;
  
  igt_main
  {
 - igt_output_t *output;
 -
   igt_skip_on_simulation();
  
   igt_fixture {
 @@ -135,10 +134,10 @@ igt_main
   igt_display_init(data.display, data.drm_fd);
   }
  
 - igt_subtest_f(flip-changes-tiling);
 -
 - for_each_connected_output(data.display, output)
 - test_flip_changes_tiling(data, output);
 + igt_subtest_f(flip-changes-tiling) {
 + for_each_connected_output(data.display, output)
 + test_flip_changes_tiling(data, output);
 + }
  
   igt_fixture {
   igt_display_fini(data.display);
 -- 
 1.8.5.2
 

-- 
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: Allow user modes to exceed DVI 165MHz limit

2014-04-11 Thread Jani Nikula

Pushed to -fixes, thanks for the patch and Daniel's r-b on IRC.

BR,
Jani.

On Thu, 27 Mar 2014, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 In commit
  commit 6375b768a9850b6154478993e5fb566fa4614a9c
  Author: Ville Syrjälä ville.syrj...@linux.intel.com
  Date:   Mon Mar 3 11:33:36 2014 +0200

 drm/i915: Reject 165MHz modes w/ DVI monitors

 the driver started to filter out display modes which exceed the
 single-link DVI 165Mz dotclock limits when the monitor doesn't report
 itself as being HDMI compliant. The intent was to filter out all
 EDID derived modes that require dual-link DVI to operate since we
 don't support dual-link.

 However the patch went a bit too far and also causes the driver to reject
 such modes even when specified by the user. Normally we don't check the
 sink limitations when setting a mode from the user. This allows the user
 to specify any mode whether the sink reports to support it or not. This
 can be useful since often the sinks support more modes than they report
 in the EDID.

 So relax the checks a bit, and apply the single-link DVI dotclock limit
 only when filtering the mode list, and ignore the limit when setting
 a user specified mode.

 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=72961
 Tested-by: Nicholas Vinson nvin...@comcast.net
 Cc: sta...@vger.kernel.org
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_hdmi.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index ee3181e..ca5d23d 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -841,11 +841,11 @@ static void intel_disable_hdmi(struct intel_encoder 
 *encoder)
   }
  }
  
 -static int hdmi_portclock_limit(struct intel_hdmi *hdmi)
 +static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool 
 respect_dvi_limit)
  {
   struct drm_device *dev = intel_hdmi_to_dev(hdmi);
  
 - if (!hdmi-has_hdmi_sink || IS_G4X(dev))
 + if ((respect_dvi_limit  !hdmi-has_hdmi_sink) || IS_G4X(dev))
   return 165000;
   else if (IS_HASWELL(dev) || INTEL_INFO(dev)-gen = 8)
   return 30;
 @@ -857,7 +857,8 @@ static enum drm_mode_status
  intel_hdmi_mode_valid(struct drm_connector *connector,
 struct drm_display_mode *mode)
  {
 - if (mode-clock  hdmi_portclock_limit(intel_attached_hdmi(connector)))
 + if (mode-clock  hdmi_portclock_limit(intel_attached_hdmi(connector),
 +true))
   return MODE_CLOCK_HIGH;
   if (mode-clock  2)
   return MODE_CLOCK_LOW;
 @@ -875,7 +876,7 @@ bool intel_hdmi_compute_config(struct intel_encoder 
 *encoder,
   struct drm_device *dev = encoder-base.dev;
   struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode;
   int clock_12bpc = pipe_config-adjusted_mode.crtc_clock * 3 / 2;
 - int portclock_limit = hdmi_portclock_limit(intel_hdmi);
 + int portclock_limit = hdmi_portclock_limit(intel_hdmi, false);
   int desired_bpp;
  
   if (intel_hdmi-color_range_auto) {
 -- 
 1.8.3.2

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

-- 
Jani Nikula, 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 0/2] Optimization on intel HDMI detect and get_modes

2014-04-11 Thread Sharma, Shashank
Ok, I will change the implementation. 

Regards
Shashank 
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Friday, April 11, 2014 7:53 PM
To: Sharma, Shashank
Cc: Daniel Vetter; C, Ramalingam; Wang, Quanxian; intel-gfx
Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and 
get_modes

On Fri, Apr 11, 2014 at 01:23:43PM +, Sharma, Shashank wrote:
 Thanks for the comments,
 Actually, we are not using live_status at all. 
 
 The check for  gen6 is only for EDID caching. So if the HW is = gen6 
 cache_edid.  
 Else do not cache EDID, so that we will not block any of the old HW, which 
 might not be HPD capable.

Oh, I've thought that this is incremental on top of something you already have.
 
 Does it sound ok now :) ?  

No. HPD is _NOT_ I repeat _NOT_ reliably. Not even on gen6+. live status simply 
reflects the hpd pin, if either doesn't work, then neither does the other one.

Nacked-with-prejudice-by: Daniel Vetter daniel.vet...@ffwll.ch

Cheers, Daniel

 
 Regards
 Shashank
 -Original Message-
 From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf 
 Of Daniel Vetter
 Sent: Friday, April 11, 2014 6:28 PM
 To: Sharma, Shashank
 Cc: C, Ramalingam; Wang, Quanxian; intel-gfx
 Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect 
 and get_modes
 
 Please don't drop the mailing list when discussing patches.
 
 And nope, conditioning this on gen6+ won't help at all, since I have a
 gen6 and gen7 machine here which don't have reliable hdmi live status.
 Afaik that is _really_ broken across the board and going with the fancy 
 invalidation scheme and delayed work items is the way forward.
 
 Cheers, Daniel
 
 On Fri, Apr 11, 2014 at 2:51 PM, Sharma, Shashank shashank.sha...@intel.com 
 wrote:
  Hi Daniel,
 
  First of all, sorry for the delay in coming up with the new patch set. I 
  got stuck with some commercial projects :) .
 
  There were few review comments what you gave for the previous optimization 
  patch, those were:
  1.  This might break the old HWs. Few of them might not even be HPD 
  capable, so its required to handle them.
  2.  Do not rely on the live_status check, as that HW is not yet proven.
  And you recommended to have a WQ, which will invalidate the cached HDMI 
  EDID after a timeout.
 
  I have written another patch, which is addressing both of the above 
  comments, but doesn't use a WQ.
 
  Before sending this to ML, I wanted to have an opinion from you, if this 
  qualifies the design you were expecting.
  I will be really glad, if you can spare some time, and just have a top 
  level view on the patch, and give your opinion.
 
  If you think that this is still not the way, and making a WQ is a better 
  approach, I would gladly create a patch which applies that.
  Or if you will prefer to have this reviewed in the mailing list only, 
  please accept my apologies for direct contact, and just let me know so. I 
  will publish this in ML.
 
  The patch is (It's also attached):
 
  From e7ec4e4827615c0ff3a4ddb020df35fdf1b0b82f Mon Sep 17 00:00:00 
  2001
  From: Shashank Sharma shashank.sha...@intel.com
  Date: Fri, 11 Apr 2014 18:02:50 +0530
  Subject: [PATCH] drm/i915: Cache HDMI EDID for future reference
 
  This patch contains following changes:
  1.Cache HDMI EDID and optimize detect() calls, which are
frequently called from userspace, causing un-necessary
EDID reads.
  2.This cached EDID will be used for detection of the status of
HDMI connector, for Non HPD detect() calls. HPD calls will update
cached EDID.
  3.This optimization is kept for new HW's (Gen6 and +), so that It
will not break old HWs who may not be even HPD capable.
 
  Signed-off-by: Shashank Sharma shashank.sha...@intel.com
  ---
   drivers/gpu/drm/i915/intel_drv.h  |1 +
   drivers/gpu/drm/i915/intel_hdmi.c |   28 ++--
   2 files changed, 27 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_drv.h
  b/drivers/gpu/drm/i915/intel_drv.h
  index 42762b7..b7911df 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -478,6 +478,7 @@ struct intel_hdmi {
  const void *frame, ssize_t len);
  void (*set_infoframes)(struct drm_encoder *encoder,
 struct drm_display_mode 
  *adjusted_mode);
  +   struct edid *edid;
   };
 
   #define DP_MAX_DOWNSTREAM_PORTS0x10
  diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
  b/drivers/gpu/drm/i915/intel_hdmi.c
  index b0413e1..33550ca 100644
  --- a/drivers/gpu/drm/i915/intel_hdmi.c
  +++ b/drivers/gpu/drm/i915/intel_hdmi.c
  @@ -938,7 +938,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool 
  force)
  hdmi_to_dig_port(intel_hdmi);
  struct intel_encoder *intel_encoder = intel_dig_port-base;
  struct drm_i915_private 

[Intel-gfx] [PATCH v2] tests/gem_error_capture: Initial testcase for error state capture/dump

2014-04-11 Thread oscar . mateo
From: Oscar Mateo oscar.ma...@intel.com

Guarantees that error capture works at a very basic level.

v2: Also check that the ring object contains a reloc with MI_BB_START
for the presumed batch object's address.

Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
 tests/.gitignore  |   1 +
 tests/Makefile.sources|   1 +
 tests/gem_error_capture.c | 269 ++
 3 files changed, 271 insertions(+)
 create mode 100644 tests/gem_error_capture.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..945574c 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -27,6 +27,7 @@ gem_ctx_create
 gem_ctx_exec
 gem_double_irq_loop
 gem_dummy_reloc_loop
+gem_error_capture
 gem_evict_alignment
 gem_evict_everything
 gem_exec_bad_domains
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index bf02a48..612beb6 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -24,6 +24,7 @@ TESTS_progs_M = \
gem_ctx_bad_exec \
gem_ctx_exec \
gem_dummy_reloc_loop \
+   gem_error_capture \
gem_evict_alignment \
gem_evict_everything \
gem_exec_bad_domains \
diff --git a/tests/gem_error_capture.c b/tests/gem_error_capture.c
new file mode 100644
index 000..88845c9
--- /dev/null
+++ b/tests/gem_error_capture.c
@@ -0,0 +1,269 @@
+/*
+ * Copyright © 2014 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.
+ *
+ * Authors:
+ *Oscar Mateo oscar.ma...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Check whether basic error state capture/dump mechanism is 
correctly
+ * working for all rings.
+ */
+
+#include unistd.h
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include drm.h
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_io.h
+#include igt_debugfs.h
+
+#define MAGIC_NUMBER 0x10001
+uint32_t batch[4] = {MI_NOOP, MI_BATCH_BUFFER_END, MAGIC_NUMBER, MAGIC_NUMBER};
+
+static void stop_rings(void)
+{
+   int fd;
+   static const char buf[] = 0xf;
+
+   fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+
+   close(fd);
+}
+
+static bool rings_stopped(void)
+{
+   int fd;
+   static char buf[128];
+   unsigned long long val;
+
+   fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(read(fd, buf, sizeof(buf))  0);
+   close(fd);
+
+   sscanf(buf, %llx, val);
+
+   return (bool)val;
+}
+
+static void clear_error_state(void)
+{
+   int fd;
+   static const char buf[] = ;
+
+   fd = igt_debugfs_open(i915_error_state, O_WRONLY);
+   igt_assert(fd = 0);
+
+   igt_assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
+   close(fd);
+}
+
+static void check_error_state(const char *expected_ring_name,
+   uint64_t expected_offset)
+{
+   FILE *file;
+   int debug_fd;
+   char *line = NULL;
+   size_t line_size = 0;
+   char *dashes = NULL;
+   char *ring_name = NULL;
+   int bb_matched = 0;
+   uint32_t gtt_offset;
+   char expected_line[32];
+   int req_matched = 0;
+   int requests;
+   uint32_t seqno, tail;
+   long jiffies;
+   int ringbuf_matched = 0;
+   unsigned int offset, command, expected_addr = 0;
+   int i, items;
+   bool bb_ok = false, req_ok = false, ringbuf_ok = false;
+
+   debug_fd = igt_debugfs_open(i915_error_state, O_RDONLY);
+   igt_assert(debug_fd = 0);
+   file = fdopen(debug_fd, r);
+
+   while (getline(line, line_size, file)  0) {
+   dashes = strstr(line, ---);
+   if (!dashes)
+   continue;
+
+   ring_name = 

[Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Jesse Barnes
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY.  This is
required on machines where the BIOS doesn't do this for us on resume to
properly re-calibrate and get the PHY ready to transmit data.

Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.

I'm open to better suggestions on how to do the power well toggle, with
the existing code it looks like I'd have to walk through a bunch of
power domains looking for a match, then call a generic function which
will warn.  I'd prefer to just expose the specific domains directly for
low level platform code like this.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_pm.c |4 ++--
 drivers/gpu/drm/i915/intel_uncore.c |   19 +++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fa00185..3afd0bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
drm_i915_private *dev_priv,
return true;
 }
 
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-  struct i915_power_well *power_well, bool enable)
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+   struct i915_power_well *power_well, bool enable)
 {
enum punit_power_well power_well_id = power_well-data;
u32 mask;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 2a72bab..f1abd2d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct drm_device 
*dev, bool restore)
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
 }
 
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+   struct i915_power_well *power_well, bool enable);
+
 void intel_uncore_early_sanitize(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
}
 
+   /*
+* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the common
+* lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+   if (IS_VALLEYVIEW(dev)) {
+   struct i915_power_well cmn_well = {
+   .data = PUNIT_POWER_WELL_DPIO_CMN_BC
+   };
+
+   vlv_set_power_well(dev_priv, cmn_well, false);
+   vlv_set_power_well(dev_priv, cmn_well, true);
+   }
+
/* clear out old GT FIFO errors */
if (IS_GEN6(dev) || IS_GEN7(dev))
__raw_i915_write32(dev_priv, GTFIFODBG,
-- 
1.7.9.5

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


[Intel-gfx] [PATCH] drm/i915/SDVO: For sysfs link put directory and target in correct order

2014-04-11 Thread Egbert Eich
When linking the i2c sysfs file into the connector's directory
pass directory and link target in the right order.
This code was introduced with:

  commit 931c1c26983b4f84e33b78579fc8d57e4a14c6b4
  Author: Imre Deak imre.d...@intel.com
  Date:   Tue Feb 11 17:12:51 2014 +0200

drm/i915: sdvo: add i2c sysfs symlink to the connector's directory

This is the same what we do for DP connectors, so make things more
consistent.

Signed-off-by: Egbert Eich e...@suse.de
---
 drivers/gpu/drm/i915/intel_sdvo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
b/drivers/gpu/drm/i915/intel_sdvo.c
index 5043f16..2d4d461 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2428,8 +2428,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector 
*connector,
if (ret  0)
goto err1;
 
-   ret = sysfs_create_link(encoder-ddc.dev.kobj,
-   drm_connector-kdev-kobj,
+   ret = sysfs_create_link(drm_connector-kdev-kobj,
+   encoder-ddc.dev.kobj,
encoder-ddc.dev.kobj.name);
if (ret  0)
goto err2;
-- 
1.8.4.5

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


Re: [Intel-gfx] [PATCH v2] tests/gem_error_capture: Initial testcase for error state capture/dump

2014-04-11 Thread Chris Wilson
On Fri, Apr 11, 2014 at 05:48:12PM +0100, oscar.ma...@intel.com wrote:
 +static void check_error_state(const char *expected_ring_name,
 + uint64_t expected_offset)
 +{
 + FILE *file;
 + int debug_fd;
 + char *line = NULL;
 + size_t line_size = 0;
 + char *dashes = NULL;
 + char *ring_name = NULL;
 + int bb_matched = 0;
 + uint32_t gtt_offset;
 + char expected_line[32];
 + int req_matched = 0;
 + int requests;
 + uint32_t seqno, tail;
 + long jiffies;
 + int ringbuf_matched = 0;
 + unsigned int offset, command, expected_addr = 0;
 + int i, items;
 + bool bb_ok = false, req_ok = false, ringbuf_ok = false;

Most of these are only of use in local scope.

 + debug_fd = igt_debugfs_open(i915_error_state, O_RDONLY);
 + igt_assert(debug_fd = 0);
 + file = fdopen(debug_fd, r);
 +
 + while (getline(line, line_size, file)  0) {
 + dashes = strstr(line, ---);
 + if (!dashes)
 + continue;
 +
 + ring_name = realloc(ring_name, dashes - line);
 + strncpy(ring_name, line, dashes - line);
 + ring_name[dashes - line - 1] = '\0';
 +
 + bb_matched = sscanf(dashes, --- gtt_offset = 0x%08x\n,
 + gtt_offset);
 + if (bb_matched == 1) {
 + igt_assert(strstr(ring_name, expected_ring_name));
 + igt_assert(gtt_offset == expected_offset);
 +
 + for (i = 0; i  sizeof(batch) / 4; i++) {
 + igt_assert(getline(line, line_size, file)  
 0);
 + snprintf(expected_line, sizeof(expected_line), 
 %08x :  %08x,
 + 4*i, batch[i]);
 + igt_assert(strstr(line, expected_line));
 + }
 + bb_ok = true;
 + continue;
 + }
 +
 + req_matched = sscanf(dashes, --- %d requests\n,
 + requests);
 + if (req_matched == 1) {
 + igt_assert(strstr(ring_name, expected_ring_name));
 + igt_assert(requests == 1);

Bad assumption. You could still have the request from
gem_quiescent_gpu() which may not have been retired before the error
triggers.

 +
 + igt_assert(getline(line, line_size, file)  0);
 + items = sscanf(line,   seqno 0x%08x, emitted %ld, tail 
 0x%08x\n,
 + seqno, jiffies, tail);
 + igt_assert(items == 3);

Bad. I may change the format. s/may/will/

 + req_ok = true;
 + continue;
 + }
 +
 + ringbuf_matched = sscanf(dashes, --- ringbuffer = 0x%08x\n,
 + gtt_offset);
 + if (ringbuf_matched == 1) {
 + if (!strstr(ring_name, expected_ring_name))
 + continue;
 + igt_assert(req_ok);
 +
 + for (i = 0; i  tail / 4; i++) {
 + igt_assert(getline(line, line_size, file)  
 0);
 + items = sscanf(line, %08x :  %08x\n,
 + offset, command);
 + igt_assert(items == 2);
 + if ((command  0x1F80) == 
 MI_BATCH_BUFFER_START) {
 + igt_assert(getline(line, line_size, 
 file)  0);
 + items = sscanf(line, %08x :  %08x\n,
 + offset, 
 expected_addr);
 + igt_assert(items == 2);
 + i++;
 + }
 + }
 + igt_assert(expected_addr == expected_offset);

Bad. Some gen encode flags into the BB start address.

 + ringbuf_ok = true;
 + continue;
 + }
 +
 + if (bb_ok  req_ok  ringbuf_ok)
 + break;
 + }
 + igt_assert(bb_ok  req_ok  ringbuf_ok);
 +
 + free(line);
 + free(ring_name);
 + close(debug_fd);
 +}

-- 
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] drm/i915/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
 This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
 that it resets the whole common lane section of the PHY.  This is
 required on machines where the BIOS doesn't do this for us on resume to
 properly re-calibrate and get the PHY ready to transmit data.
 
 Without this patch, such machines won't resume correctly much of the time,
 with the symptom being a 'port ready' timeout and/or a link training
 failure.
 
 I'm open to better suggestions on how to do the power well toggle, with
 the existing code it looks like I'd have to walk through a bunch of
 power domains looking for a match, then call a generic function which
 will warn.  I'd prefer to just expose the specific domains directly for
 low level platform code like this.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_pm.c |4 ++--
  drivers/gpu/drm/i915/intel_uncore.c |   19 +++
  2 files changed, 21 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index fa00185..3afd0bc 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
 drm_i915_private *dev_priv,
   return true;
  }
  
 -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 -struct i915_power_well *power_well, bool enable)
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable)
  {
   enum punit_power_well power_well_id = power_well-data;
   u32 mask;
 diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
 b/drivers/gpu/drm/i915/intel_uncore.c
 index 2a72bab..f1abd2d 100644
 --- a/drivers/gpu/drm/i915/intel_uncore.c
 +++ b/drivers/gpu/drm/i915/intel_uncore.c
 @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
 drm_device *dev, bool restore)
   spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
  }
  
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable);
 +
  void intel_uncore_early_sanitize(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
   DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
   }
  
 + /*
 +  * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
 +  * Need to assert and de-assert PHY SB reset by gating the common
 +  * lane power, then un-gating it.
 +  * Simply ungating isn't enough to reset the PHY enough to get
 +  * ports and lanes running.
 +  */
 + if (IS_VALLEYVIEW(dev)) {
 + struct i915_power_well cmn_well = {
 + .data = PUNIT_POWER_WELL_DPIO_CMN_BC
 + };
 +
 + vlv_set_power_well(dev_priv, cmn_well, false);
 + vlv_set_power_well(dev_priv, cmn_well, true);
 + }

Relationship with intel_reset_dpio? Should we move this bit of code over
there? I'm lost in this maze of kick-me-harder patches for byt dpio ...
-Daniel

 +
   /* clear out old GT FIFO errors */
   if (IS_GEN6(dev) || IS_GEN7(dev))
   __raw_i915_write32(dev_priv, GTFIFODBG,
 -- 
 1.7.9.5
 
 ___
 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] drm/i915/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Imre Deak
On Fri, 2014-04-11 at 10:00 -0700, Jesse Barnes wrote:
 This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
 that it resets the whole common lane section of the PHY.  This is
 required on machines where the BIOS doesn't do this for us on resume to
 properly re-calibrate and get the PHY ready to transmit data.
 
 Without this patch, such machines won't resume correctly much of the time,
 with the symptom being a 'port ready' timeout and/or a link training
 failure.
 
 I'm open to better suggestions on how to do the power well toggle, with
 the existing code it looks like I'd have to walk through a bunch of
 power domains looking for a match, then call a generic function which
 will warn.  I'd prefer to just expose the specific domains directly for
 low level platform code like this.

The power_well-sync_hw() handler looks like a good place for such
things. It will get called from intel_power_domains_init_hw(), which is
later than then the uncore sanitize functions, but then again if it's
really needed that early then intel_power_domains_init_hw() should be
moved earlier too..

--Imre

 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_pm.c |4 ++--
  drivers/gpu/drm/i915/intel_uncore.c |   19 +++
  2 files changed, 21 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index fa00185..3afd0bc 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
 drm_i915_private *dev_priv,
   return true;
  }
  
 -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 -struct i915_power_well *power_well, bool enable)
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable)
  {
   enum punit_power_well power_well_id = power_well-data;
   u32 mask;
 diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
 b/drivers/gpu/drm/i915/intel_uncore.c
 index 2a72bab..f1abd2d 100644
 --- a/drivers/gpu/drm/i915/intel_uncore.c
 +++ b/drivers/gpu/drm/i915/intel_uncore.c
 @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
 drm_device *dev, bool restore)
   spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
  }
  
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable);
 +
  void intel_uncore_early_sanitize(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
   DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
   }
  
 + /*
 +  * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
 +  * Need to assert and de-assert PHY SB reset by gating the common
 +  * lane power, then un-gating it.
 +  * Simply ungating isn't enough to reset the PHY enough to get
 +  * ports and lanes running.
 +  */
 + if (IS_VALLEYVIEW(dev)) {
 + struct i915_power_well cmn_well = {
 + .data = PUNIT_POWER_WELL_DPIO_CMN_BC
 + };
 +
 + vlv_set_power_well(dev_priv, cmn_well, false);
 + vlv_set_power_well(dev_priv, cmn_well, true);
 + }
 +
   /* clear out old GT FIFO errors */
   if (IS_GEN6(dev) || IS_GEN7(dev))
   __raw_i915_write32(dev_priv, GTFIFODBG,



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/SDVO: For sysfs link put directory and target in correct order

2014-04-11 Thread Imre Deak
On Fri, 2014-04-11 at 19:07 +0200, Egbert Eich wrote:
 When linking the i2c sysfs file into the connector's directory
 pass directory and link target in the right order.
 This code was introduced with:
 
   commit 931c1c26983b4f84e33b78579fc8d57e4a14c6b4
   Author: Imre Deak imre.d...@intel.com
   Date:   Tue Feb 11 17:12:51 2014 +0200
 
 drm/i915: sdvo: add i2c sysfs symlink to the connector's directory
 
 This is the same what we do for DP connectors, so make things more
 consistent.
 
 Signed-off-by: Egbert Eich e...@suse.de

Oops, good catch. The fix looks ok, fwiw:
Reviewed-by: Imre Deak imre.d...@intel.com

 ---
  drivers/gpu/drm/i915/intel_sdvo.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
 b/drivers/gpu/drm/i915/intel_sdvo.c
 index 5043f16..2d4d461 100644
 --- a/drivers/gpu/drm/i915/intel_sdvo.c
 +++ b/drivers/gpu/drm/i915/intel_sdvo.c
 @@ -2428,8 +2428,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector 
 *connector,
   if (ret  0)
   goto err1;
  
 - ret = sysfs_create_link(encoder-ddc.dev.kobj,
 - drm_connector-kdev-kobj,
 + ret = sysfs_create_link(drm_connector-kdev-kobj,
 + encoder-ddc.dev.kobj,
   encoder-ddc.dev.kobj.name);
   if (ret  0)
   goto err2;



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Ville Syrjälä
On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
 This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
 that it resets the whole common lane section of the PHY.  This is
 required on machines where the BIOS doesn't do this for us on resume to
 properly re-calibrate and get the PHY ready to transmit data.
 
 Without this patch, such machines won't resume correctly much of the time,
 with the symptom being a 'port ready' timeout and/or a link training
 failure.
 
 I'm open to better suggestions on how to do the power well toggle, with
 the existing code it looks like I'd have to walk through a bunch of
 power domains looking for a match, then call a generic function which
 will warn.  I'd prefer to just expose the specific domains directly for
 low level platform code like this.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_pm.c |4 ++--
  drivers/gpu/drm/i915/intel_uncore.c |   19 +++
  2 files changed, 21 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index fa00185..3afd0bc 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
 drm_i915_private *dev_priv,
   return true;
  }
  
 -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 -struct i915_power_well *power_well, bool enable)
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable)
  {
   enum punit_power_well power_well_id = power_well-data;
   u32 mask;
 diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
 b/drivers/gpu/drm/i915/intel_uncore.c
 index 2a72bab..f1abd2d 100644
 --- a/drivers/gpu/drm/i915/intel_uncore.c
 +++ b/drivers/gpu/drm/i915/intel_uncore.c
 @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
 drm_device *dev, bool restore)
   spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
  }
  
 +void vlv_set_power_well(struct drm_i915_private *dev_priv,
 + struct i915_power_well *power_well, bool enable);
 +
  void intel_uncore_early_sanitize(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
   DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
   }
  
 + /*
 +  * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
 +  * Need to assert and de-assert PHY SB reset by gating the common
 +  * lane power, then un-gating it.
 +  * Simply ungating isn't enough to reset the PHY enough to get
 +  * ports and lanes running.
 +  */
 + if (IS_VALLEYVIEW(dev)) {
 + struct i915_power_well cmn_well = {
 + .data = PUNIT_POWER_WELL_DPIO_CMN_BC
 + };
 +
 + vlv_set_power_well(dev_priv, cmn_well, false);
 + vlv_set_power_well(dev_priv, cmn_well, true);
 + }

Stick this into intel_reset_dpio() instead?

And what about fastboot and whatnot? Should we check if the display is
already up and running somehow before we go and kill it with this?

 +
   /* clear out old GT FIFO errors */
   if (IS_GEN6(dev) || IS_GEN7(dev))
   __raw_i915_write32(dev_priv, GTFIFODBG,
 -- 
 1.7.9.5
 
 ___
 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/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Jesse Barnes
On Fri, 11 Apr 2014 19:16:32 +0200
Daniel Vetter dan...@ffwll.ch wrote:

 On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
  This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
  that it resets the whole common lane section of the PHY.  This is
  required on machines where the BIOS doesn't do this for us on resume to
  properly re-calibrate and get the PHY ready to transmit data.
  
  Without this patch, such machines won't resume correctly much of the time,
  with the symptom being a 'port ready' timeout and/or a link training
  failure.
  
  I'm open to better suggestions on how to do the power well toggle, with
  the existing code it looks like I'd have to walk through a bunch of
  power domains looking for a match, then call a generic function which
  will warn.  I'd prefer to just expose the specific domains directly for
  low level platform code like this.
  
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_pm.c |4 ++--
   drivers/gpu/drm/i915/intel_uncore.c |   19 +++
   2 files changed, 21 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index fa00185..3afd0bc 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
  drm_i915_private *dev_priv,
  return true;
   }
   
  -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
  -  struct i915_power_well *power_well, bool enable)
  +void vlv_set_power_well(struct drm_i915_private *dev_priv,
  +   struct i915_power_well *power_well, bool enable)
   {
  enum punit_power_well power_well_id = power_well-data;
  u32 mask;
  diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
  b/drivers/gpu/drm/i915/intel_uncore.c
  index 2a72bab..f1abd2d 100644
  --- a/drivers/gpu/drm/i915/intel_uncore.c
  +++ b/drivers/gpu/drm/i915/intel_uncore.c
  @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
  drm_device *dev, bool restore)
  spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
   }
   
  +void vlv_set_power_well(struct drm_i915_private *dev_priv,
  +   struct i915_power_well *power_well, bool enable);
  +
   void intel_uncore_early_sanitize(struct drm_device *dev)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device 
  *dev)
  DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
  }
   
  +   /*
  +* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
  +* Need to assert and de-assert PHY SB reset by gating the common
  +* lane power, then un-gating it.
  +* Simply ungating isn't enough to reset the PHY enough to get
  +* ports and lanes running.
  +*/
  +   if (IS_VALLEYVIEW(dev)) {
  +   struct i915_power_well cmn_well = {
  +   .data = PUNIT_POWER_WELL_DPIO_CMN_BC
  +   };
  +
  +   vlv_set_power_well(dev_priv, cmn_well, false);
  +   vlv_set_power_well(dev_priv, cmn_well, true);
  +   }
 
 Relationship with intel_reset_dpio? Should we move this bit of code over
 there? I'm lost in this maze of kick-me-harder patches for byt dpio ...

That happens too late.  This will clobber register state, whereas the
DPIO reset just resets the interface between the phy and the display.

-- 
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/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Jesse Barnes
On Fri, 11 Apr 2014 20:26:24 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:

 On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
  This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
  that it resets the whole common lane section of the PHY.  This is
  required on machines where the BIOS doesn't do this for us on resume to
  properly re-calibrate and get the PHY ready to transmit data.
  
  Without this patch, such machines won't resume correctly much of the time,
  with the symptom being a 'port ready' timeout and/or a link training
  failure.
  
  I'm open to better suggestions on how to do the power well toggle, with
  the existing code it looks like I'd have to walk through a bunch of
  power domains looking for a match, then call a generic function which
  will warn.  I'd prefer to just expose the specific domains directly for
  low level platform code like this.
  
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_pm.c |4 ++--
   drivers/gpu/drm/i915/intel_uncore.c |   19 +++
   2 files changed, 21 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index fa00185..3afd0bc 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct 
  drm_i915_private *dev_priv,
  return true;
   }
   
  -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
  -  struct i915_power_well *power_well, bool enable)
  +void vlv_set_power_well(struct drm_i915_private *dev_priv,
  +   struct i915_power_well *power_well, bool enable)
   {
  enum punit_power_well power_well_id = power_well-data;
  u32 mask;
  diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
  b/drivers/gpu/drm/i915/intel_uncore.c
  index 2a72bab..f1abd2d 100644
  --- a/drivers/gpu/drm/i915/intel_uncore.c
  +++ b/drivers/gpu/drm/i915/intel_uncore.c
  @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
  drm_device *dev, bool restore)
  spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
   }
   
  +void vlv_set_power_well(struct drm_i915_private *dev_priv,
  +   struct i915_power_well *power_well, bool enable);
  +
   void intel_uncore_early_sanitize(struct drm_device *dev)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device 
  *dev)
  DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
  }
   
  +   /*
  +* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
  +* Need to assert and de-assert PHY SB reset by gating the common
  +* lane power, then un-gating it.
  +* Simply ungating isn't enough to reset the PHY enough to get
  +* ports and lanes running.
  +*/
  +   if (IS_VALLEYVIEW(dev)) {
  +   struct i915_power_well cmn_well = {
  +   .data = PUNIT_POWER_WELL_DPIO_CMN_BC
  +   };
  +
  +   vlv_set_power_well(dev_priv, cmn_well, false);
  +   vlv_set_power_well(dev_priv, cmn_well, true);
  +   }
 
 Stick this into intel_reset_dpio() instead?
 
 And what about fastboot and whatnot? Should we check if the display is
 already up and running somehow before we go and kill it with this?

reset_dpio is too late.

But yes, this generally doesn't need to happen on the boot path (well
maybe on some platforms) so we should do something conditional there or
we'll lose all the display state.

-- 
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: Intel-specific primary plane handling (v2)

2014-04-11 Thread Matt Roper
On Fri, Apr 11, 2014 at 11:34:36AM +0200, Daniel Vetter wrote:
 On Thu, Apr 10, 2014 at 05:24:36PM -0700, Matt Roper wrote:
...
 
 Hm, I've thought we could do a simple
 
   if (intel_crtc-primary_enabled)
   call_primary_plane_helper
   else
   enable_the_hw_plane
 
 But we need to do all the arg checking for the !primary_enabled case :(
 Anyway more code sharing make me happier.
 
 Cheers, Daniel

I think the problem here is that the helper has a bunch of tests
targetted at the lowest common denominator hardware.  Some of the things
it rejects are things that our hardware may begin to allow at some point
in the future (e.g., primary plane scaling, partial CRTC coverage of
primary plane, etc.).  We can probably call into the helper today and
get the behavior we want, but I'd expect that some of those restrictions
will need to be relaxed in the future and then we'll have to switch the
code back at that point.  Given that we still need to do all this
checking in the 'if (!enabled)' case, I don't think it's worth trying to
call through the helper for the 'if (enabled)' case (especially since
the actual work here after we're done testing is just a couple lines
of code)?


Matt


-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Ville Syrjälä
On Fri, Apr 11, 2014 at 10:34:19AM -0700, Jesse Barnes wrote:
 On Fri, 11 Apr 2014 19:16:32 +0200
 Daniel Vetter dan...@ffwll.ch wrote:
 
  On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
   This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
   that it resets the whole common lane section of the PHY.  This is
   required on machines where the BIOS doesn't do this for us on resume to
   properly re-calibrate and get the PHY ready to transmit data.
   
   Without this patch, such machines won't resume correctly much of the time,
   with the symptom being a 'port ready' timeout and/or a link training
   failure.
   
   I'm open to better suggestions on how to do the power well toggle, with
   the existing code it looks like I'd have to walk through a bunch of
   power domains looking for a match, then call a generic function which
   will warn.  I'd prefer to just expose the specific domains directly for
   low level platform code like this.
   
   Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
   ---
drivers/gpu/drm/i915/intel_pm.c |4 ++--
drivers/gpu/drm/i915/intel_uncore.c |   19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index fa00185..3afd0bc 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -5454,8 +5454,8 @@ static bool 
   i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 return true;
}

   -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
   -struct i915_power_well *power_well, bool enable)
   +void vlv_set_power_well(struct drm_i915_private *dev_priv,
   + struct i915_power_well *power_well, bool enable)
{
 enum punit_power_well power_well_id = power_well-data;
 u32 mask;
   diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
   b/drivers/gpu/drm/i915/intel_uncore.c
   index 2a72bab..f1abd2d 100644
   --- a/drivers/gpu/drm/i915/intel_uncore.c
   +++ b/drivers/gpu/drm/i915/intel_uncore.c
   @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
   drm_device *dev, bool restore)
 spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}

   +void vlv_set_power_well(struct drm_i915_private *dev_priv,
   + struct i915_power_well *power_well, bool enable);
   +
void intel_uncore_early_sanitize(struct drm_device *dev)
{
 struct drm_i915_private *dev_priv = dev-dev_private;
   @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device 
   *dev)
 DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
 }

   + /*
   +  * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
   +  * Need to assert and de-assert PHY SB reset by gating the common
   +  * lane power, then un-gating it.
   +  * Simply ungating isn't enough to reset the PHY enough to get
   +  * ports and lanes running.
   +  */
   + if (IS_VALLEYVIEW(dev)) {
   + struct i915_power_well cmn_well = {
   + .data = PUNIT_POWER_WELL_DPIO_CMN_BC
   + };
   +
   + vlv_set_power_well(dev_priv, cmn_well, false);
   + vlv_set_power_well(dev_priv, cmn_well, true);
   + }
  
  Relationship with intel_reset_dpio? Should we move this bit of code over
  there? I'm lost in this maze of kick-me-harder patches for byt dpio ...
 
 That happens too late.  This will clobber register state, whereas the
 DPIO reset just resets the interface between the phy and the display.

As a clarification to the cmnreset thing, we never actually assert
that signal, we just deassert it. The idea being that it should be
asserted by default when things get powered on. But I wonder if we
should assert it before suspending anyway.

Oh and I think if we power gate the cmnlane we would need to
assert/deassert cmnreset around it. In some CHV doc I see a note
that side reset must be deasserted before cmnreset. The timing
diagrams in VLV docs seem to have that order as well. So unless
there's some internal logic which hold cmnreset asserted for the
required time, we should do it by hand.


Oh and there's another intersting looking note:

NOTE1 : Common lane reset must not be de-asserted until REFCLK to PLL is
 enabled by i_pll*refclkbufen and the clock is running and stable

I guess we managed to follow that by accident since we enable the
refclock for DPLLB for the hotplug workaround. But perhaps we should
enable the refclk for all PLLs just to be sure.

-- 
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/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Ville Syrjälä
On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
 On Fri, 11 Apr 2014 20:26:24 +0300
 Ville Syrjälä ville.syrj...@linux.intel.com wrote:
 
  On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
   This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
   that it resets the whole common lane section of the PHY.  This is
   required on machines where the BIOS doesn't do this for us on resume to
   properly re-calibrate and get the PHY ready to transmit data.
   
   Without this patch, such machines won't resume correctly much of the time,
   with the symptom being a 'port ready' timeout and/or a link training
   failure.
   
   I'm open to better suggestions on how to do the power well toggle, with
   the existing code it looks like I'd have to walk through a bunch of
   power domains looking for a match, then call a generic function which
   will warn.  I'd prefer to just expose the specific domains directly for
   low level platform code like this.
   
   Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
   ---
drivers/gpu/drm/i915/intel_pm.c |4 ++--
drivers/gpu/drm/i915/intel_uncore.c |   19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index fa00185..3afd0bc 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -5454,8 +5454,8 @@ static bool 
   i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 return true;
}

   -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
   -struct i915_power_well *power_well, bool enable)
   +void vlv_set_power_well(struct drm_i915_private *dev_priv,
   + struct i915_power_well *power_well, bool enable)
{
 enum punit_power_well power_well_id = power_well-data;
 u32 mask;
   diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
   b/drivers/gpu/drm/i915/intel_uncore.c
   index 2a72bab..f1abd2d 100644
   --- a/drivers/gpu/drm/i915/intel_uncore.c
   +++ b/drivers/gpu/drm/i915/intel_uncore.c
   @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
   drm_device *dev, bool restore)
 spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
}

   +void vlv_set_power_well(struct drm_i915_private *dev_priv,
   + struct i915_power_well *power_well, bool enable);
   +
void intel_uncore_early_sanitize(struct drm_device *dev)
{
 struct drm_i915_private *dev_priv = dev-dev_private;
   @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device 
   *dev)
 DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
 }

   + /*
   +  * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
   +  * Need to assert and de-assert PHY SB reset by gating the common
   +  * lane power, then un-gating it.
   +  * Simply ungating isn't enough to reset the PHY enough to get
   +  * ports and lanes running.
   +  */
   + if (IS_VALLEYVIEW(dev)) {
   + struct i915_power_well cmn_well = {
   + .data = PUNIT_POWER_WELL_DPIO_CMN_BC
   + };
   +
   + vlv_set_power_well(dev_priv, cmn_well, false);
   + vlv_set_power_well(dev_priv, cmn_well, true);
   + }
  
  Stick this into intel_reset_dpio() instead?
  
  And what about fastboot and whatnot? Should we check if the display is
  already up and running somehow before we go and kill it with this?
 
 reset_dpio is too late.

How come? We shouldn't touch the PHY before it. So either reset_dpio is
in the wrong place for some reason, or something else gets called too
soon.

-- 
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/vlv: assert and de-assert sideband reset on resume

2014-04-11 Thread Jesse Barnes
On Fri, 11 Apr 2014 21:10:21 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:

 On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
  On Fri, 11 Apr 2014 20:26:24 +0300
  Ville Syrjälä ville.syrj...@linux.intel.com wrote:
  
   On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY.  This is
required on machines where the BIOS doesn't do this for us on resume to
properly re-calibrate and get the PHY ready to transmit data.

Without this patch, such machines won't resume correctly much of the 
time,
with the symptom being a 'port ready' timeout and/or a link training
failure.

I'm open to better suggestions on how to do the power well toggle, with
the existing code it looks like I'd have to walk through a bunch of
power domains looking for a match, then call a generic function which
will warn.  I'd prefer to just expose the specific domains directly for
low level platform code like this.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_pm.c |4 ++--
 drivers/gpu/drm/i915/intel_uncore.c |   19 +++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c 
b/drivers/gpu/drm/i915/intel_pm.c
index fa00185..3afd0bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5454,8 +5454,8 @@ static bool 
i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
return true;
 }
 
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-  struct i915_power_well *power_well, bool 
enable)
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+   struct i915_power_well *power_well, bool enable)
 {
enum punit_power_well power_well_id = power_well-data;
u32 mask;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 2a72bab..f1abd2d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct 
drm_device *dev, bool restore)
spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
 }
 
+void vlv_set_power_well(struct drm_i915_private *dev_priv,
+   struct i915_power_well *power_well, bool 
enable);
+
 void intel_uncore_early_sanitize(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device 
*dev)
DRM_INFO(Found %zuMB of eLLC\n, dev_priv-ellc_size);
}
 
+   /*
+* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the 
common
+* lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+   if (IS_VALLEYVIEW(dev)) {
+   struct i915_power_well cmn_well = {
+   .data = PUNIT_POWER_WELL_DPIO_CMN_BC
+   };
+
+   vlv_set_power_well(dev_priv, cmn_well, false);
+   vlv_set_power_well(dev_priv, cmn_well, true);
+   }
   
   Stick this into intel_reset_dpio() instead?
   
   And what about fastboot and whatnot? Should we check if the display is
   already up and running somehow before we go and kill it with this?
  
  reset_dpio is too late.
 
 How come? We shouldn't touch the PHY before it. So either reset_dpio is
 in the wrong place for some reason, or something else gets called too
 soon.

Oh actually I haven't tested with just the common reset, it may be ok
to put that into the DPIO init function.  My earlier patch was toggling
all the wells, including display, which would obviously clobber things.

-- 
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: Intel-specific primary plane handling (v2)

2014-04-11 Thread Daniel Vetter
On Fri, Apr 11, 2014 at 10:41:56AM -0700, Matt Roper wrote:
 On Fri, Apr 11, 2014 at 11:34:36AM +0200, Daniel Vetter wrote:
  On Thu, Apr 10, 2014 at 05:24:36PM -0700, Matt Roper wrote:
 ...
  
  Hm, I've thought we could do a simple
  
  if (intel_crtc-primary_enabled)
  call_primary_plane_helper
  else
  enable_the_hw_plane
  
  But we need to do all the arg checking for the !primary_enabled case :(
  Anyway more code sharing make me happier.
  
  Cheers, Daniel
 
 I think the problem here is that the helper has a bunch of tests
 targetted at the lowest common denominator hardware.  Some of the things
 it rejects are things that our hardware may begin to allow at some point
 in the future (e.g., primary plane scaling, partial CRTC coverage of
 primary plane, etc.).  We can probably call into the helper today and
 get the behavior we want, but I'd expect that some of those restrictions
 will need to be relaxed in the future and then we'll have to switch the
 code back at that point.  Given that we still need to do all this
 checking in the 'if (!enabled)' case, I don't think it's worth trying to
 call through the helper for the 'if (enabled)' case (especially since
 the actual work here after we're done testing is just a couple lines
 of code)?

Well for that future I simply expect that we'll get a completely new
update_plane function. I agree that reusing the helper completely doesn't
work really, but sharing the tests would be nice imo.
-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] drm/i915: Intel-specific primary plane handling (v3)

2014-04-11 Thread Matt Roper
Intel hardware allows the primary plane to be disabled independently of
the CRTC.  Provide custom primary plane handling to allow this.

v3:
 - Provide gen-specific primary plane format lists (suggested by Daniel
   Vetter).
 - If the primary plane is already enabled, go ahead and just call the
   primary plane helper to do the update (suggested by Daniel Vetter).
 - Don't try to disable the primary plane on destruction; the DRM layer
   should have already taken care of this for us.
v2:
 - Unpin fb properly on primary plane disable
 - Provide an Intel-specific set of primary plane formats
 - Additional sanity checks on setplane (in line with the checks
   currently being done by the DRM core primary plane helper)

Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 197 ++-
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 2 files changed, 196 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5..3e4d36a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -39,8 +39,35 @@
 #include i915_trace.h
 #include drm/drm_dp_helper.h
 #include drm/drm_crtc_helper.h
+#include drm/drm_plane_helper.h
+#include drm/drm_rect.h
 #include linux/dma_remapping.h
 
+/* Primary plane formats supported by all gen */
+#define COMMON_PRIMARY_FORMATS \
+   DRM_FORMAT_C8, \
+   DRM_FORMAT_RGB565, \
+   DRM_FORMAT_XRGB, \
+   DRM_FORMAT_ARGB
+
+/* Primary plane formats for gen = 3 */
+const static uint32_t intel_primary_formats_gen3[] = {
+   COMMON_PRIMARY_FORMATS,
+   DRM_FORMAT_XRGB1555,
+   DRM_FORMAT_ARGB1555,
+};
+
+/* Primary plane formats for gen = 4 */
+const static uint32_t intel_primary_formats_gen4[] = {
+   COMMON_PRIMARY_FORMATS, \
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_XRGB2101010,
+   DRM_FORMAT_ARGB2101010,
+   DRM_FORMAT_XBGR2101010,
+   DRM_FORMAT_ABGR2101010,
+};
+
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 
@@ -10556,17 +10583,183 @@ static void intel_shared_dpll_init(struct drm_device 
*dev)
BUG_ON(dev_priv-num_shared_dpll  I915_NUM_PLLS);
 }
 
+static int
+intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
+struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+unsigned int crtc_w, unsigned int crtc_h,
+uint32_t src_x, uint32_t src_y,
+uint32_t src_w, uint32_t src_h)
+{
+   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_framebuffer *tmpfb;
+   struct drm_rect dest = {
+   .x1 = crtc_x,
+   .y1 = crtc_y,
+   .x2 = crtc_x + crtc_w,
+   .y2 = crtc_y + crtc_h,
+   };
+   struct drm_rect clip = {
+   .x2 = crtc-mode.hdisplay,
+   .y2 = crtc-mode.vdisplay,
+   };
+   int ret;
+
+   /*
+* At the moment we use the same set of setplane restrictions as the
+* DRM primary plane helper, so go ahead and just call the helper if
+* the primary plane is already enabled.  We only need to take special
+* action if the primary plane is disabled (something i915 can do but
+* the generic helper can't).
+*/
+   if (intel_crtc-primary_enabled)
+   return drm_primary_helper_update(plane, crtc, fb,
+crtc_x, crtc_y,
+crtc_w, crtc_h,
+src_x, src_y,
+src_w, src_h);
+
+   /* setplane API takes shifted source rectangle values; unshift them */
+   src_x = 16;
+   src_y = 16;
+   src_w = 16;
+   src_h = 16;
+
+   /*
+* Current hardware can't reposition the primary plane or scale it
+* (although this could change in the future).
+*/
+   drm_rect_intersect(dest, clip);
+   if (dest.x1 != 0 || dest.y1 != 0 ||
+   dest.x2 != crtc-mode.hdisplay || dest.y2 != crtc-mode.vdisplay) {
+   DRM_DEBUG_KMS(Primary plane must cover entire CRTC\n);
+   return -EINVAL;
+   }
+
+   if (crtc_w != src_w || crtc_h != src_h) {
+   DRM_DEBUG_KMS(Can't scale primary plane\n);
+   return -EINVAL;
+   }
+
+   /* Primary planes are locked to their owning CRTC */
+   if (plane-possible_crtcs != drm_crtc_mask(crtc)) {
+   DRM_DEBUG_KMS(Cannot change primary plane CRTC\n);
+   return -EINVAL;
+   }
+
+   /* Framebuffer 

[Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v4)

2014-04-11 Thread Matt Roper
Intel hardware allows the primary plane to be disabled independently of
the CRTC.  Provide custom primary plane handling to allow this.

v4:
 - Don't add a primary_plane field to intel_crtc; that was left over
   from a much earlier iteration of this patch series, but is no longer
   needed/used now that the DRM core primary plane support has been
   merged.
v3:
 - Provide gen-specific primary plane format lists (suggested by Daniel
   Vetter).
 - If the primary plane is already enabled, go ahead and just call the
   primary plane helper to do the update (suggested by Daniel Vetter).
 - Don't try to disable the primary plane on destruction; the DRM layer
   should have already taken care of this for us.
v2:
 - Unpin fb properly on primary plane disable
 - Provide an Intel-specific set of primary plane formats
 - Additional sanity checks on setplane (in line with the checks
   currently being done by the DRM core primary plane helper)

Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 197 ++-
 1 file changed, 195 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5..3e4d36a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -39,8 +39,35 @@
 #include i915_trace.h
 #include drm/drm_dp_helper.h
 #include drm/drm_crtc_helper.h
+#include drm/drm_plane_helper.h
+#include drm/drm_rect.h
 #include linux/dma_remapping.h
 
+/* Primary plane formats supported by all gen */
+#define COMMON_PRIMARY_FORMATS \
+   DRM_FORMAT_C8, \
+   DRM_FORMAT_RGB565, \
+   DRM_FORMAT_XRGB, \
+   DRM_FORMAT_ARGB
+
+/* Primary plane formats for gen = 3 */
+const static uint32_t intel_primary_formats_gen3[] = {
+   COMMON_PRIMARY_FORMATS,
+   DRM_FORMAT_XRGB1555,
+   DRM_FORMAT_ARGB1555,
+};
+
+/* Primary plane formats for gen = 4 */
+const static uint32_t intel_primary_formats_gen4[] = {
+   COMMON_PRIMARY_FORMATS, \
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_XRGB2101010,
+   DRM_FORMAT_ARGB2101010,
+   DRM_FORMAT_XBGR2101010,
+   DRM_FORMAT_ABGR2101010,
+};
+
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 
@@ -10556,17 +10583,183 @@ static void intel_shared_dpll_init(struct drm_device 
*dev)
BUG_ON(dev_priv-num_shared_dpll  I915_NUM_PLLS);
 }
 
+static int
+intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
+struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+unsigned int crtc_w, unsigned int crtc_h,
+uint32_t src_x, uint32_t src_y,
+uint32_t src_w, uint32_t src_h)
+{
+   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_framebuffer *tmpfb;
+   struct drm_rect dest = {
+   .x1 = crtc_x,
+   .y1 = crtc_y,
+   .x2 = crtc_x + crtc_w,
+   .y2 = crtc_y + crtc_h,
+   };
+   struct drm_rect clip = {
+   .x2 = crtc-mode.hdisplay,
+   .y2 = crtc-mode.vdisplay,
+   };
+   int ret;
+
+   /*
+* At the moment we use the same set of setplane restrictions as the
+* DRM primary plane helper, so go ahead and just call the helper if
+* the primary plane is already enabled.  We only need to take special
+* action if the primary plane is disabled (something i915 can do but
+* the generic helper can't).
+*/
+   if (intel_crtc-primary_enabled)
+   return drm_primary_helper_update(plane, crtc, fb,
+crtc_x, crtc_y,
+crtc_w, crtc_h,
+src_x, src_y,
+src_w, src_h);
+
+   /* setplane API takes shifted source rectangle values; unshift them */
+   src_x = 16;
+   src_y = 16;
+   src_w = 16;
+   src_h = 16;
+
+   /*
+* Current hardware can't reposition the primary plane or scale it
+* (although this could change in the future).
+*/
+   drm_rect_intersect(dest, clip);
+   if (dest.x1 != 0 || dest.y1 != 0 ||
+   dest.x2 != crtc-mode.hdisplay || dest.y2 != crtc-mode.vdisplay) {
+   DRM_DEBUG_KMS(Primary plane must cover entire CRTC\n);
+   return -EINVAL;
+   }
+
+   if (crtc_w != src_w || crtc_h != src_h) {
+   DRM_DEBUG_KMS(Can't scale primary plane\n);
+   return -EINVAL;
+   }
+
+   /* Primary planes are locked to their owning CRTC */
+   if 

[Intel-gfx] [PATCH 2/2] drm/i915: remove misplaced panel wait in DP off code

2014-04-11 Thread Jesse Barnes
We do this wait elsewhere, so drop it to speed things up a bit.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_dp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 728a5db..7642415 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2787,7 +2787,6 @@ intel_dp_link_down(struct intel_dp *intel_dp)
DP = ~DP_AUDIO_OUTPUT_ENABLE;
I915_WRITE(intel_dp-output_reg, DP  ~DP_PORT_EN);
POSTING_READ(intel_dp-output_reg);
-   msleep(intel_dp-panel_power_down_delay);
 }
 
 static bool
-- 
1.8.4.2

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


[Intel-gfx] [PATCH 1/2] drm/i915: remove unexplained vblank wait in the DP off code

2014-04-11 Thread Jesse Barnes
I don't think this is necessary; at least it doesn't appear to be on my
BYT.  Dropping it speeds up our shutdown code a little, in some cases
resulting in faster init times.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_dp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e48d47c..728a5db 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2756,9 +2756,6 @@ intel_dp_link_down(struct intel_dp *intel_dp)
}
POSTING_READ(intel_dp-output_reg);
 
-   /* We don't really know why we're doing this */
-   intel_wait_for_vblank(dev, intel_crtc-pipe);
-
if (HAS_PCH_IBX(dev) 
I915_READ(intel_dp-output_reg)  DP_PIPEB_SELECT) {
struct drm_crtc *crtc = intel_dig_port-base.base.crtc;
-- 
1.8.4.2

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


[Intel-gfx] [PATCH] drm/i915/bdw: cs-stall before state cache invld w/a

2014-04-11 Thread Ben Widawsky
We do this already for previous GENs. I guess we must do it for BDW too
according to DOCS.

Pipe_control with CS-stall bit set must be issued before a
pipe-control command that has the State Cache Invalidate bit set.

This does not solve the problem I have unfortunately.

I didn't check if this was in Ville's CHV series. If it was, I
apologize.

NOTE: I tried to use smaller lengths for the command, but nothing made
it happy except 6.

Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Jordan Justen jljus...@gmail.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a9b04d1..092dea0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -266,17 +266,25 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
 static int
 gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 {
-   int ret;
+   int ret, size = 4;
 
-   ret = intel_ring_begin(ring, 4);
+   if (IS_BROADWELL(ring-dev))
+   size += 2;
+
+   ret = intel_ring_begin(ring, size);
if (ret)
return ret;
 
-   intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
+   intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(size));
intel_ring_emit(ring, PIPE_CONTROL_CS_STALL |
  PIPE_CONTROL_STALL_AT_SCOREBOARD);
intel_ring_emit(ring, 0);
intel_ring_emit(ring, 0);
+   if (IS_BROADWELL(ring-dev)) {
+   intel_ring_emit(ring, 0);
+   intel_ring_emit(ring, 0);
+   }
+
intel_ring_advance(ring);
 
return 0;
@@ -389,6 +397,11 @@ gen8_render_ring_flush(struct intel_ring_buffer *ring,
flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_QW_WRITE;
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+
+   /* Workaround: we must issue a pipe_control with CS-stall bit
+* set before a pipe_control command that has the state cache
+* invalidate bit set. */
+   gen7_render_ring_cs_stall_wa(ring);
}
 
ret = intel_ring_begin(ring, 6);
-- 
1.9.1

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