Re: [Intel-gfx] Responsiveness Changes to i915 Driver

2014-08-15 Thread Chris Wilson
On Thu, Aug 14, 2014 at 11:52:47PM +, Wilde, Martin wrote:
 Greetings,
 
 I am submitting the below changes to i915 Gfx driver to support resume time 
 Responsiveness measurements.  These changes parallel the work already done in 
 the IVB Windows Gfx driver.  These changes in addition to other OTS scripts 
 (suspend_resume) allow tracking of what is referred to as the “B2I” or 
 “Button To Image” time of the platform.  The shorter this time, the more 
 responsiveness the platform is viewed by the end user.  Panel selection is an 
 important factor in providing a more responsive system.  Note there is no 
 dependency on other scripts.  The changes are standalone.
 
 
   *   Display the current T1_T3 value.  This is used to verify that the 
 timing set in the VBT is correct.  We have seen many instances where the 
 value is not set correctly for the panel and the resume time is longer than 
 necessary (e.g. 500ms T3 versus 200ms T3).
   *   Print the time when the first page flip occurs.  This is when the user 
 first sees the desktop displayed from resume.  While this measurement could 
 be done by other methods, this is the actual time that the desktop 
 manager/framebuffer makes the driver request and the Gfx driver performs the 
 action.  Thus any layering software added can be correlated to increases in 
 this time.
 
 To support the latter (first page flip), I added a new ftrace called 
 “trace_i915_resume”.  I looked at the existing page flip trace message and 
 that one is designed for every page flip.  I did not want to convolute it 
 with the one time flip trace on resume.  I used a trace message instead of a 
 printk to reduce any performance impacts of using a printk.  Additionally 
 printk is not reliable of when the message actually appears in the kernel log.

I am sorry, you are complaining that there is a tracepoint that gives
you exactly what you want already, only that userspace needs to do some
filtering? Which by the way, does not give you what you say you want
anyway - the scanout is already active long before the first flip is
handled, and in many, many cass that flip is just a figment of your
imagination. Maybe what you mean is to B2UR rather than
button-to-static-image.
-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 v2] drm/i915: Don't warn if we restore pm interrupts during reset

2014-08-15 Thread Chris Wilson
On Thu, Aug 14, 2014 at 05:45:59PM +0300, Ville Syrjälä wrote:
 On Thu, Aug 14, 2014 at 04:23:16PM +0200, Daniel Vetter wrote:
  On Thu, Aug 14, 2014 at 03:46:43PM +0300, Mika Kuoppala wrote:
   We lost the software state tracking due to reset, so don't
   complain if it doesn't match.
  
  This sounds more like gpu reset should be a bit more careful (even more
  careful than we already are compared to earlier kernels) with making sure
  the irq state is still sane after a reset?
  
  Or what exactly is the failure mode here? The commit message lacks a bit
  details in form of a nice text or even better: A testcase ;-)
 
 Killing the hpd irq and gt_powersave junk from i915_reset() would be my
 suggestion here. I don't even know why the hpd stuff is still there, we
 removed all the other irq frobbery from there a while back. And last I
 looked gpu reset didn't affect the rc6/rps stuff either, though more
 testing should be done to make sure I didn't just imagine it.

I'd like a call to intel_mark_idle() since we are resetting the GPU
activity tracker. That is meant to dtrt with all the powersave tracking
as well. (Well if someone would just review some patches it might dtrt!)
-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 00/16] drm/i915: 830M/ns201 fixes again

2014-08-15 Thread Ville Syrjälä
On Fri, Aug 15, 2014 at 01:21:52AM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 Thomas asked me to repost my 830/ns2501 patches. So here they are. I added
 a few more patches (trickle feed and unused ring init) to fix some post-resume
 issues. The primary plane rmw elimination patches and some locking/load detect
 fixes already got merged.
 
 Apart from these we still lack the minimum watermark check. I guess
 we could just take the patch Thomas posted [1]. Doesn't look like a
 more advanced solution is coming any time soon. Though the commit
 message of that patch needs work and it lacks a s-o-b.
 
 The VGACNTR patch might not be necessary any longer since Daniel's
 vga/dummycon stuff. I don't recall if I tested without it, but my
 gut feeling is that it's no longer needed. But I included the patch
 here anyway.
 
 [1] http://patchwork.freedesktop.org/patch/27318/

Here's the branch (includes my earlier watermark hack):

git://gitorious.org/vsyrjala/linux.git alm_fixes11

-- 
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: fix plane/cursor handling when runtime suspended

2014-08-15 Thread Ville Syrjälä
On Thu, Aug 14, 2014 at 12:06:02PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 If we're runtime suspended and try to use the plane interfaces, we
 will get a lot of WARNs saying we did the wrong thing.
 
 We need to get runtime PM references to pin the objects, and to
 change the fences. The pin functions are the ideal places for
 this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
 have to add get/put calls inside it. There is no problem if we runtime
 suspend right after these functions are finished, because the
 registers written are forwarded to system memory.
 
 Note: for a complete fix of the cursor-dpms test case, we also need
 the patch named drm/i915: Don't try to enable cursor from setplane
 when crtc is disabled.
 
 v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
 v3: - Make get/put also surround the fence and unpin calls (Daniel and
   Ville).
 - Merge all the plane changes into a single patch since they're
   the same fix.
 - Add the comment requested by Daniel.
 v4: - Remove spurious whitespace (Ville).
 v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
   equivalent fix in another patch (Ville).
 v6: - Remove unpin chunk: it will be on a separate patch (Ville,
   Chris, Daniel).
 
 Testcase: igt/pm_rpm/cursor
 Testcase: igt/pm_rpm/cursor-dpms
 Testcase: igt/pm_rpm/legacy-planes
 Testcase: igt/pm_rpm/legacy-planes-dpms
 Testcase: igt/pm_rpm/universal-planes
 Testcase: igt/pm_rpm/universal-planes-dpms
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
 Cc: sta...@vger.kernel.org
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 27 +++
  1 file changed, 27 insertions(+)
 
 
 I talked with Daniel and we agreed to leave any possible fixes related with 
 the
 unpin functions to separate patches, possibly requiring separate IGT test
 cases.
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 3813526..17bc661 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
   if (need_vtd_wa(dev)  alignment  256 * 1024)
   alignment = 256 * 1024;
  
 + /*
 +  * Global gtt pte registers are special registers which actually forward
 +  * writes to a chunk of system memory. Which means that there is no risk
 +  * that the register values disappear as soon as we call
 +  * intel_runtime_pm_put(), so it is correct to wrap only the
 +  * pin/unpin/fence and not more.
 +  */
 + intel_runtime_pm_get(dev_priv);
 +
   dev_priv-mm.interruptible = false;
   ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
   if (ret)
 @@ -2166,12 +2175,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
   i915_gem_object_pin_fence(obj);
  
   dev_priv-mm.interruptible = true;
 + intel_runtime_pm_put(dev_priv);
   return 0;
  
  err_unpin:
   i915_gem_object_unpin_from_display_plane(obj);
  err_interruptible:
   dev_priv-mm.interruptible = true;
 + intel_runtime_pm_put(dev_priv);
   return ret;
  }
  
 @@ -8201,6 +8212,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
 *crtc,
uint32_t width, uint32_t height)
  {
   struct drm_device *dev = crtc-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   enum pipe pipe = intel_crtc-pipe;
   unsigned old_width, stride;
 @@ -8231,6 +8243,16 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
 *crtc,
  
   /* we only need to pin inside GTT if cursor is non-phy */
   mutex_lock(dev-struct_mutex);
 +
 + /*
 +  * Global gtt pte registers are special registers which actually forward
 +  * writes to a chunk of system memory. Which means that there is no risk
 +  * that the register values disappear as soon as we call
 +  * intel_runtime_pm_put(), so it is correct to wrap only the
 +  * pin/unpin/fence and not more.
 +  */
 + intel_runtime_pm_get(dev_priv);
 +

Only the !cursor_needs_physical case need runtime pm get/put. I thought
the calls were there originally, but I guess they got moved out. I
suppose it's not a huge deal either way, but the current approach does
give the reader the wrong impression that the unpin and frontbuffer
tracking would also need a rpm reference.

   if (!INTEL_INFO(dev)-cursor_needs_physical) {
   unsigned alignment;
  
 @@ -8280,6 +8302,10 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
 *crtc,
  
   i915_gem_track_fb(intel_crtc-cursor_bo, obj,
 INTEL_FRONTBUFFER_CURSOR(pipe));
 +
 + if (obj)
 + intel_runtime_pm_put(dev_priv);
 +
   

Re: [Intel-gfx] [PATCH 27/43] drm/i915/bdw: Render state init for Execlists

2014-08-15 Thread Daniel, Thomas


 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
 Vetter
 Sent: Thursday, August 14, 2014 9:00 PM
 To: Daniel, Thomas
 Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 27/43] drm/i915/bdw: Render state init for
 Execlists
 
 On Wed, Aug 13, 2014 at 05:30:07PM +0200, Daniel Vetter wrote:
  On Wed, Aug 13, 2014 at 03:07:29PM +, Daniel, Thomas wrote:
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of
Daniel Vetter
Sent: Monday, August 11, 2014 10:25 PM
To: Daniel, Thomas
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 27/43] drm/i915/bdw: Render state
init for Execlists
   
On Thu, Jul 24, 2014 at 05:04:35PM +0100, Thomas Daniel wrote:
 From: Oscar Mateo oscar.ma...@intel.com
 index 9085ff1..0dc6992 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -513,8 +513,23 @@ int i915_gem_context_enable(struct
drm_i915_private *dev_priv)
   ppgtt-enable(ppgtt);
   }

 - if (i915.enable_execlists)
 + if (i915.enable_execlists) {
 + struct intel_context *dctx;
 +
 + ring = dev_priv-ring[RCS];
 + dctx = ring-default_context;
 +
 + if (!dctx-rcs_initialized) {
 + ret = intel_lr_context_render_state_init(ring,
 dctx);
 + if (ret) {
 + DRM_ERROR(Init render state failed:
 %d\n,
ret);
 + return ret;
 + }
 + dctx-rcs_initialized = true;
 + }
 +
   return 0;
 + }
   
This looks very much like the wrong place. We should init the
render state when we create the context, or when we switch to it for
 the first time.
The later is what the legacy contexts currently do in do_switch.
   
But ctx_enable should do the switch to the default context and
that's about
   Well, a side-effect of switching to the default context in legacy
   mode is that the render state gets initialized.  I could move the lr
   render state init call into an enable_execlists branch in
   i915_switch_context() but that doen't seem like the right place.
  
   How about in i915_gem_init() after calling i915_gem_init_hw()?
  
it. If there's some depency then I guess we should stall the
creation of the default context a bit, maybe.
   
In any case someone needs to explain this better and if there's
not other wey this at least needs a bit comment. So I'll punt for now.
   When the default context is created the driver is not ready to
   execute a batch.  That is why the render state init can't be done then.
 
  That sounds like the default context is created too early. Essentially
  I want to avoid needless divergence between the default context and
  normal contexts, because sooner or later that will means someone will
  creep in with a _really_ subtle bug.
 
  What about:
  - We create the default lrc contexs in context_init, but like with a
normal context we don't do any of the deferred setup.
  - In context_enable (which since yesterday properly propagates errors to
callers) we force the deferred lrc ctx setup for the default contexts on
all engines.
  - The render state init is done as part of the deferred ctx setup for the
render engine in all cases.
 
  Totally off the track or do you see a workable solution somewhere in
  that direction?
 
 I'd like to discuss this first a bit more, so will punt on this patch for now.
 -Daniel
I think that your proposal will work.  I've been having some trouble
with my RVP board so haven't had a chance to test it out yet.

Thomas.
 --
 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 30/43] drm/i915/bdw: Two-stage execlist submit process

2014-08-15 Thread Daniel, Thomas


 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
 Vetter
 Sent: Thursday, August 14, 2014 9:10 PM
 To: Daniel, Thomas
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 30/43] drm/i915/bdw: Two-stage execlist
 submit process
 
 On Thu, Jul 24, 2014 at 05:04:38PM +0100, Thomas Daniel wrote:
  From: Michel Thierry michel.thie...@intel.com
 
  Context switch (and execlist submission) should happen only when other
  contexts are not active, otherwise pre-emption occurs.
 
  To assure this, we place context switch requests in a queue and those
  request are later consumed when the right context switch interrupt is
  received (still TODO).
 
  v2: Use a spinlock, do not remove the requests on unqueue (wait for
  context switch completion).
 
  Signed-off-by: Thomas Daniel thomas.dan...@intel.com
 
  v3: Several rebases and code changes. Use unique ID.
 
  v4:
  - Move the queue/lock init to the late ring initialization.
  - Damien's kmalloc review comments: check return, use sizeof(*req), do
  not cast.
 
  v5:
  - Do not reuse drm_i915_gem_request. Instead, create our own.
  - New namespace.
 
  Signed-off-by: Michel Thierry michel.thie...@intel.com (v1)
  Signed-off-by: Oscar Mateo oscar.ma...@intel.com (v2-v5)
  ---
   drivers/gpu/drm/i915/intel_lrc.c|   63
 ++-
   drivers/gpu/drm/i915/intel_lrc.h|8 
   drivers/gpu/drm/i915/intel_ringbuffer.h |2 +
   3 files changed, 71 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_lrc.c
  b/drivers/gpu/drm/i915/intel_lrc.c
  index 5b6f416..9e91169 100644
  --- a/drivers/gpu/drm/i915/intel_lrc.c
  +++ b/drivers/gpu/drm/i915/intel_lrc.c
  @@ -217,6 +217,63 @@ static int execlists_submit_context(struct
 intel_engine_cs *ring,
  return 0;
   }
 
  +static void execlists_context_unqueue(struct intel_engine_cs *ring) {
  +   struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
  +   struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
  +
  +   if (list_empty(ring-execlist_queue))
  +   return;
  +
  +   /* Try to read in pairs */
  +   list_for_each_entry_safe(cursor, tmp, ring-execlist_queue,
  +execlist_link) {
 
 Ok, because checkpatch I've looked at this. Imo open-coding this would be
 much easier to read i.e.
 
   if (!list_empty)
   grabremove first item;
   if (!list_empty)
   grabremove 2nd item;
 
 Care to follow up with a patch for that?
 
 Thanks, Daniel
This needs to be kept as a loop because if there are two consecutive
requests for the same context they are squashed.  Also the non-squashed
requests are not removed here (unfortunately the remove is in the next
patch).

 
  +   if (!req0)
  +   req0 = cursor;
  +   else if (req0-ctx == cursor-ctx) {
This:

  +   /* Same ctx: ignore first request, as second request
  +* will update tail past first request's workload */
  +   list_del(req0-execlist_link);
  +   i915_gem_context_unreference(req0-ctx);
  +   kfree(req0);
  +   req0 = cursor;
  +   } else {
  +   req1 = cursor;
  +   break;
  +   }
  +   }
  +
  +   BUG_ON(execlists_submit_context(ring, req0-ctx, req0-tail,
  +   req1? req1-ctx : NULL, req1? req1-tail : 0)); }
  +
  +static int execlists_context_queue(struct intel_engine_cs *ring,
  +  struct intel_context *to,
  +  u32 tail)
  +{
  +   struct intel_ctx_submit_request *req = NULL;
  +   unsigned long flags;
  +   bool was_empty;
  +
  +   req = kzalloc(sizeof(*req), GFP_KERNEL);
  +   if (req == NULL)
  +   return -ENOMEM;
  +   req-ctx = to;
  +   i915_gem_context_reference(req-ctx);
  +   req-ring = ring;
  +   req-tail = tail;
  +
  +   spin_lock_irqsave(ring-execlist_lock, flags);
  +
  +   was_empty = list_empty(ring-execlist_queue);
  +   list_add_tail(req-execlist_link, ring-execlist_queue);
  +   if (was_empty)
  +   execlists_context_unqueue(ring);
  +
  +   spin_unlock_irqrestore(ring-execlist_lock, flags);
  +
  +   return 0;
  +}
  +
   static int logical_ring_invalidate_all_caches(struct intel_ringbuffer
  *ringbuf)  {
  struct intel_engine_cs *ring = ringbuf-ring; @@ -405,8 +462,7 @@
  void intel_logical_ring_advance_and_submit(struct intel_ringbuffer
 *ringbuf)
  if (intel_ring_stopped(ring))
  return;
 
  -   /* FIXME: too cheeky, we don't even check if the ELSP is ready */
  -   execlists_submit_context(ring, ctx, ringbuf-tail, NULL, 0);
  +   execlists_context_queue(ring, ctx, ringbuf-tail);
   }
 
   static int logical_ring_alloc_seqno(struct intel_engine_cs *ring, @@
  -850,6 +906,9 @@ static int logical_ring_init(struct drm_device *dev, struct
 intel_engine_cs *rin
  

Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

2014-08-15 Thread Imre Deak
On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
 The series seems fine to me.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
 for the rest as well.

Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
existed even in 3.16, but only the MST support made it apparent with the
extra HPD signaling and DP AUX activity during suspend.

--Imre 


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


Re: [Intel-gfx] [PATCH 30/43] drm/i915/bdw: Two-stage execlist submit process

2014-08-15 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 08:51:22AM +, Daniel, Thomas wrote:
  -Original Message-
  From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
  Vetter
  Sent: Thursday, August 14, 2014 9:10 PM
  To: Daniel, Thomas
  Cc: intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH 30/43] drm/i915/bdw: Two-stage execlist
  submit process
  On Thu, Jul 24, 2014 at 05:04:38PM +0100, Thomas Daniel wrote:
   From: Michel Thierry michel.thie...@intel.com
   +static void execlists_context_unqueue(struct intel_engine_cs *ring) {
   + struct intel_ctx_submit_request *req0 = NULL, *req1 = NULL;
   + struct intel_ctx_submit_request *cursor = NULL, *tmp = NULL;
   +
   + if (list_empty(ring-execlist_queue))
   + return;
   +
   + /* Try to read in pairs */
   + list_for_each_entry_safe(cursor, tmp, ring-execlist_queue,
   +execlist_link) {
  
  Ok, because checkpatch I've looked at this. Imo open-coding this would be
  much easier to read i.e.
  
  if (!list_empty)
  grabremove first item;
  if (!list_empty)
  grabremove 2nd item;
  
  Care to follow up with a patch for that?
  
  Thanks, Daniel
 This needs to be kept as a loop because if there are two consecutive
 requests for the same context they are squashed.  Also the non-squashed
 requests are not removed here (unfortunately the remove is in the next
 patch).

Ok, this sounds like we need to overhaul it anyway for the request
tracking then.
-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: fix plane rotation when restoring fbdev configuration

2014-08-15 Thread Thomas Wood
On 14 August 2014 17:02, Daniel Vetter dan...@ffwll.ch wrote:
 On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote:
 Make sure plane rotation is reset correctly when restoring the fbdev
 configuration by using drm_mode_plane_set_obj_prop. This calls the
 driver's set_property callback and ensures the rotation is actually
 changed.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236
 Signed-off-by: Thomas Wood thomas.w...@intel.com

 Commit message is missing the citation of the offending commit that
 introduced this. With that addressed this is

 Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch

 And please cc all the people involved in the offending commit next time
 around, too.

The reset feature was introduced in commit 9783de2 (drm: Resetting
rotation property), although it also looks like the issue was
originally addressed in a previous version of the patch:

http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html

Although this version calls the driver's set_property directly rather
than exporting drm_mode_plane_set_obj_prop.

The final version of the patch contains a different change:



 -Daniel
 ---
  drivers/gpu/drm/drm_crtc.c  | 25 -
  drivers/gpu/drm/drm_fb_helper.c |  6 +++---
  include/drm/drm_crtc.h  |  3 +++
  3 files changed, 26 insertions(+), 8 deletions(-)

 diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
 index f09b752..95f330a 100644
 --- a/drivers/gpu/drm/drm_crtc.c
 +++ b/drivers/gpu/drm/drm_crtc.c
 @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct 
 drm_mode_object *obj,
   return ret;
  }

 -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
 -   struct drm_property *property,
 -   uint64_t value)
 +/**
 + * drm_mode_plane_set_obj_prop - set the value of a property
 + * @plane: drm plane object to set property value for
 + * @property: property to set
 + * @val: value the property should be set to
 + *
 + * This functions sets a given property on a given plane object. This 
 function
 + * calls the driver's -set_property callback and changes the software 
 state of
 + * the property if the callback succeeds.
 + *
 + * Returns:
 + * Zero on success, error code on failure.
 + */
 +int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 + struct drm_property *property,
 + uint64_t value)
  {
   int ret = -EINVAL;
 - struct drm_plane *plane = obj_to_plane(obj);
 + struct drm_mode_object *obj = plane-base;

   if (plane-funcs-set_property)
   ret = plane-funcs-set_property(plane, property, value);
 @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct 
 drm_mode_object *obj,

   return ret;
  }
 +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);

  /**
   * drm_mode_getproperty_ioctl - get the current value of a object's property
 @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
 *dev, void *data,
   ret = drm_mode_crtc_set_obj_prop(arg_obj, property, 
 arg-value);
   break;
   case DRM_MODE_OBJECT_PLANE:
 - ret = drm_mode_plane_set_obj_prop(arg_obj, property, 
 arg-value);
 + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj),
 +   property, arg-value);
   break;
   }

 diff --git a/drivers/gpu/drm/drm_fb_helper.c 
 b/drivers/gpu/drm/drm_fb_helper.c
 index 63d7b8e..0c0c39b 100644
 --- a/drivers/gpu/drm/drm_fb_helper.c
 +++ b/drivers/gpu/drm/drm_fb_helper.c
 @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
 *fb_helper)
   drm_plane_force_disable(plane);

   if (dev-mode_config.rotation_property) {
 - drm_object_property_set_value(plane-base,
 - dev-mode_config.rotation_property,
 - BIT(DRM_ROTATE_0));
 + drm_mode_plane_set_obj_prop(plane,
 + 
 dev-mode_config.rotation_property,
 + BIT(DRM_ROTATE_0));
   }
   }

 diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
 index 0375d75..31344bf 100644
 --- a/include/drm/drm_crtc.h
 +++ b/include/drm/drm_crtc.h
 @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct 
 drm_device *dev, void *data,
struct drm_file *file_priv);
  extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void 
 *data,
  struct drm_file *file_priv);
 +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 +struct drm_property *property,
 +uint64_t value);

  

Re: [Intel-gfx] [PATCH] drm: fix plane rotation when restoring fbdev configuration

2014-08-15 Thread Thomas Wood
On 15 August 2014 10:42, Thomas Wood thomas.w...@intel.com wrote:
 On 14 August 2014 17:02, Daniel Vetter dan...@ffwll.ch wrote:
 On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote:
 Make sure plane rotation is reset correctly when restoring the fbdev
 configuration by using drm_mode_plane_set_obj_prop. This calls the
 driver's set_property callback and ensures the rotation is actually
 changed.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236
 Signed-off-by: Thomas Wood thomas.w...@intel.com

 Commit message is missing the citation of the offending commit that
 introduced this. With that addressed this is

 Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch

 And please cc all the people involved in the offending commit next time
 around, too.

 The reset feature was introduced in commit 9783de2 (drm: Resetting
 rotation property), although it also looks like the issue was
 originally addressed in a previous version of the patch:

 http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html

 Although this version calls the driver's set_property directly rather
 than exporting drm_mode_plane_set_obj_prop.

 The final version of the patch contains a different change:

http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html

So there were actually two different v2 versions of the patch.




 -Daniel
 ---
  drivers/gpu/drm/drm_crtc.c  | 25 -
  drivers/gpu/drm/drm_fb_helper.c |  6 +++---
  include/drm/drm_crtc.h  |  3 +++
  3 files changed, 26 insertions(+), 8 deletions(-)

 diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
 index f09b752..95f330a 100644
 --- a/drivers/gpu/drm/drm_crtc.c
 +++ b/drivers/gpu/drm/drm_crtc.c
 @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct 
 drm_mode_object *obj,
   return ret;
  }

 -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
 -   struct drm_property *property,
 -   uint64_t value)
 +/**
 + * drm_mode_plane_set_obj_prop - set the value of a property
 + * @plane: drm plane object to set property value for
 + * @property: property to set
 + * @val: value the property should be set to
 + *
 + * This functions sets a given property on a given plane object. This 
 function
 + * calls the driver's -set_property callback and changes the software 
 state of
 + * the property if the callback succeeds.
 + *
 + * Returns:
 + * Zero on success, error code on failure.
 + */
 +int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 + struct drm_property *property,
 + uint64_t value)
  {
   int ret = -EINVAL;
 - struct drm_plane *plane = obj_to_plane(obj);
 + struct drm_mode_object *obj = plane-base;

   if (plane-funcs-set_property)
   ret = plane-funcs-set_property(plane, property, value);
 @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct 
 drm_mode_object *obj,

   return ret;
  }
 +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);

  /**
   * drm_mode_getproperty_ioctl - get the current value of a object's 
 property
 @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
 *dev, void *data,
   ret = drm_mode_crtc_set_obj_prop(arg_obj, property, 
 arg-value);
   break;
   case DRM_MODE_OBJECT_PLANE:
 - ret = drm_mode_plane_set_obj_prop(arg_obj, property, 
 arg-value);
 + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj),
 +   property, arg-value);
   break;
   }

 diff --git a/drivers/gpu/drm/drm_fb_helper.c 
 b/drivers/gpu/drm/drm_fb_helper.c
 index 63d7b8e..0c0c39b 100644
 --- a/drivers/gpu/drm/drm_fb_helper.c
 +++ b/drivers/gpu/drm/drm_fb_helper.c
 @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
 *fb_helper)
   drm_plane_force_disable(plane);

   if (dev-mode_config.rotation_property) {
 - drm_object_property_set_value(plane-base,
 - dev-mode_config.rotation_property,
 - BIT(DRM_ROTATE_0));
 + drm_mode_plane_set_obj_prop(plane,
 + 
 dev-mode_config.rotation_property,
 + BIT(DRM_ROTATE_0));
   }
   }

 diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
 index 0375d75..31344bf 100644
 --- a/include/drm/drm_crtc.h
 +++ b/include/drm/drm_crtc.h
 @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct 
 drm_device *dev, void *data,
struct drm_file *file_priv);
  extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void 
 *data,
  struct 

Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

2014-08-15 Thread Jani Nikula
On Fri, 15 Aug 2014, Imre Deak imre.d...@intel.com wrote:
 On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
 The series seems fine to me.
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
 for the rest as well.

 Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
 existed even in 3.16, but only the MST support made it apparent with the
 extra HPD signaling and DP AUX activity during suspend.

The series no longer applies on any branch. Would you mind respinning on
-fixes please?

BR,
Jani.



 --Imre 



-- 
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] Usage of _PAGE_PCD et al in i915 driver

2014-08-15 Thread Juergen Gross

On 08/13/2014 05:07 PM, Jesse Barnes wrote:

On Fri, 8 Aug 2014 15:14:15 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:


Adding relevant mailing lists.

On Fri, Aug 8, 2014 at 1:23 PM, Juergen Gross jgr...@suse.com wrote:

I'm just about to create a patch for full PAT support in the Linux
kernel, including Xen. For this purpose I introduce a translation
between cache modes and pte bits.

Scanning the kernel sources for usage of the cache mode bits in the
pte I discovered  drivers/gpu/drm/i915/i915_gem_gtt.h is using
_PAGE_PCD, _PAGE_PWT and _PAGE_PAT. I think those defines are used
to create ptes not for usage by the main processor, but for the
graphics processor. Is this true? In this case I'd suggest to define
i915-specific macros instead of using the x86 ones.


Yeah, those are gpu specific PAT tables, but the hw engineers
specifically designed this to match, and we've tried to follow the cpu
side to match it. Especially in the future that will be somewhat
important, since we want to fully share the entire address space
between cpu and gpu on the next platform. Jesse is working on that.


Right, we have an x86 compatible MMU in the GPU itself, so re-using the
defines makes sense.  I suppose with your work you'll move them and
make them a bit more opaque?  If so, we'll still want a way to get at
them directly, or access your mapping functions for generating PTE bits
for the GPU MMU.


Using the mapping functions I'm introducing should work, if the MMU has
an x86 compatible MSR_IA32_CR_PAT which is configured the same way as
on the x86 processor (be aware that Xen is using another MSR_IA32_CR_PAT
setting as the Linux kernel).

Juergen

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


Re: [Intel-gfx] [PATCH 11/43] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-15 Thread Daniel, Thomas


 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
 Vetter
 Sent: Monday, August 11, 2014 3:30 PM
 To: Daniel, Thomas
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH 11/43] drm/i915/bdw: Render moot context
 reset and switch with Execlists
 
 On Thu, Jul 24, 2014 at 05:04:19PM +0100, Thomas Daniel wrote:
  From: Oscar Mateo oscar.ma...@intel.com
 
  These two functions make no sense in an Logical Ring Context 
  Execlists world.
 
  v2: We got rid of lrc_enabled and centralized everything in the
  sanitized i915.enbale_execlists instead.
 
  Signed-off-by: Oscar Mateo oscar.ma...@intel.com
  ---
   drivers/gpu/drm/i915/i915_gem_context.c |9 +
   1 file changed, 9 insertions(+)
 
  diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
  b/drivers/gpu/drm/i915/i915_gem_context.c
  index fbe7278..288f5de 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -380,6 +380,9 @@ void i915_gem_context_reset(struct drm_device
 *dev)
  struct drm_i915_private *dev_priv = dev-dev_private;
  int i;
 
  +   if (i915.enable_execlists)
  +   return;
 
 This will conflict badly with Alistair's patch at a functional level. I'm 
 pretty sure
 that we want _some_ form of reset for the context state, since the hw didn't
 just magically load the previously running context. So NACK on this hunk.
OK I'll wait to see the final version of Alistair's patch and decide what to do
about this hunk.

 
  +
  /* Prevent the hardware from restoring the last context (which
 hung) on
   * the next switch */
  for (i = 0; i  I915_NUM_RINGS; i++) { @@ -514,6 +517,9 @@ int
  i915_gem_context_enable(struct drm_i915_private *dev_priv)
  ppgtt-enable(ppgtt);
  }
 
  +   if (i915.enable_execlists)
  +   return 0;
 
 Again this conflicts with Alistair's patch. Furthermore it looks redudant 
 since
 you no-op out i915_switch_context separately.
I don't think this is a conflict.  Doesn't Alistair's change here just involve
writing PDPs for full PPGTT?  We don't want to do that in lrc mode.

 
  +
  /* FIXME: We should make this work, even in reset */
  if (i915_reset_in_progress(dev_priv-gpu_error))
  return 0;
  @@ -769,6 +775,9 @@ int i915_switch_context(struct intel_engine_cs
  *ring,  {
  struct drm_i915_private *dev_priv = ring-dev-dev_private;
 
  +   if (i915.enable_execlists)
  +   return 0;
 
 I've hoped we don't need this with the legacy ringbuffer cmdsubmission fuly
 split out. If there are other paths (resume, gpu reset) where this comes into
 play then I guess we need to look at where the best place is to make this 
 call.
 So until this comes with a bit a better justification I'll punt on this for 
 now.
 -Daniel
Yes, the command submission lrc path doesn't call this but other codepaths
do.  If we keep the check in context_enable() the only remaining call I see is
in i915_gpu_idle().  I don't mind if the check is done there but perhaps a
WARN_ON should then be added into switch_context() because we don't
want to be putting illegal MI_SET_CONTEXT commands into the ring.

Thomas.

  +
  WARN_ON(!mutex_is_locked(dev_priv-dev-struct_mutex));
 
  if (to-legacy_hw_ctx.rcs_state == NULL) { /* We have the fake
  context */
  --
  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] Usage of _PAGE_PCD et al in i915 driver

2014-08-15 Thread Ville Syrjälä
On Thu, Aug 14, 2014 at 05:55:11AM +0200, Juergen Gross wrote:
 On 08/13/2014 05:07 PM, Jesse Barnes wrote:
  On Fri, 8 Aug 2014 15:14:15 +0200
  Daniel Vetter daniel.vet...@ffwll.ch wrote:
 
  Adding relevant mailing lists.
 
  On Fri, Aug 8, 2014 at 1:23 PM, Juergen Gross jgr...@suse.com wrote:
  I'm just about to create a patch for full PAT support in the Linux
  kernel, including Xen. For this purpose I introduce a translation
  between cache modes and pte bits.
 
  Scanning the kernel sources for usage of the cache mode bits in the
  pte I discovered  drivers/gpu/drm/i915/i915_gem_gtt.h is using
  _PAGE_PCD, _PAGE_PWT and _PAGE_PAT. I think those defines are used
  to create ptes not for usage by the main processor, but for the
  graphics processor. Is this true? In this case I'd suggest to define
  i915-specific macros instead of using the x86 ones.
 
  Yeah, those are gpu specific PAT tables, but the hw engineers
  specifically designed this to match, and we've tried to follow the cpu
  side to match it. Especially in the future that will be somewhat
  important, since we want to fully share the entire address space
  between cpu and gpu on the next platform. Jesse is working on that.
 
  Right, we have an x86 compatible MMU in the GPU itself, so re-using the
  defines makes sense.  I suppose with your work you'll move them and
  make them a bit more opaque?  If so, we'll still want a way to get at
  them directly, or access your mapping functions for generating PTE bits
  for the GPU MMU.
 
 Using the mapping functions I'm introducing should work, if the MMU has
 an x86 compatible MSR_IA32_CR_PAT which is configured the same way as
 on the x86 processor (be aware that Xen is using another MSR_IA32_CR_PAT
 setting as the Linux kernel).

We have a PAT that is structured the same way as the x86 PAT. But the
contents of the PAT entries are obviously specific to the GPU so it's
not identical. But the pcd/pwt/pat bits index the PAT in exactly the
same way as on x86.

See bdw_setup_private_ppat() and chv_setup_private_ppat() for how we
set up the PAT.

-- 
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: fix plane rotation when restoring fbdev configuration

2014-08-15 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 10:48:05AM +0100, Thomas Wood wrote:
 On 15 August 2014 10:42, Thomas Wood thomas.w...@intel.com wrote:
  On 14 August 2014 17:02, Daniel Vetter dan...@ffwll.ch wrote:
  On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote:
  Make sure plane rotation is reset correctly when restoring the fbdev
  configuration by using drm_mode_plane_set_obj_prop. This calls the
  driver's set_property callback and ensures the rotation is actually
  changed.
 
  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236
  Signed-off-by: Thomas Wood thomas.w...@intel.com
 
  Commit message is missing the citation of the offending commit that
  introduced this. With that addressed this is
 
  Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 
  And please cc all the people involved in the offending commit next time
  around, too.
 
  The reset feature was introduced in commit 9783de2 (drm: Resetting
  rotation property), although it also looks like the issue was
  originally addressed in a previous version of the patch:
 
  http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html
 
  Although this version calls the driver's set_property directly rather
  than exporting drm_mode_plane_set_obj_prop.
 
  The final version of the patch contains a different change:
 
 http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html
 
 So there were actually two different v2 versions of the patch.

Somehow I've thought this is for -fixes, but the patch is only in dinq. So
merged, thanks.
-Daniel

 
 
 
 
  -Daniel
  ---
   drivers/gpu/drm/drm_crtc.c  | 25 -
   drivers/gpu/drm/drm_fb_helper.c |  6 +++---
   include/drm/drm_crtc.h  |  3 +++
   3 files changed, 26 insertions(+), 8 deletions(-)
 
  diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
  index f09b752..95f330a 100644
  --- a/drivers/gpu/drm/drm_crtc.c
  +++ b/drivers/gpu/drm/drm_crtc.c
  @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct 
  drm_mode_object *obj,
return ret;
   }
 
  -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
  -   struct drm_property *property,
  -   uint64_t value)
  +/**
  + * drm_mode_plane_set_obj_prop - set the value of a property
  + * @plane: drm plane object to set property value for
  + * @property: property to set
  + * @val: value the property should be set to
  + *
  + * This functions sets a given property on a given plane object. This 
  function
  + * calls the driver's -set_property callback and changes the software 
  state of
  + * the property if the callback succeeds.
  + *
  + * Returns:
  + * Zero on success, error code on failure.
  + */
  +int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
  + struct drm_property *property,
  + uint64_t value)
   {
int ret = -EINVAL;
  - struct drm_plane *plane = obj_to_plane(obj);
  + struct drm_mode_object *obj = plane-base;
 
if (plane-funcs-set_property)
ret = plane-funcs-set_property(plane, property, value);
  @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct 
  drm_mode_object *obj,
 
return ret;
   }
  +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
 
   /**
* drm_mode_getproperty_ioctl - get the current value of a object's 
  property
  @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct 
  drm_device *dev, void *data,
ret = drm_mode_crtc_set_obj_prop(arg_obj, property, 
  arg-value);
break;
case DRM_MODE_OBJECT_PLANE:
  - ret = drm_mode_plane_set_obj_prop(arg_obj, property, 
  arg-value);
  + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj),
  +   property, arg-value);
break;
}
 
  diff --git a/drivers/gpu/drm/drm_fb_helper.c 
  b/drivers/gpu/drm/drm_fb_helper.c
  index 63d7b8e..0c0c39b 100644
  --- a/drivers/gpu/drm/drm_fb_helper.c
  +++ b/drivers/gpu/drm/drm_fb_helper.c
  @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
  *fb_helper)
drm_plane_force_disable(plane);
 
if (dev-mode_config.rotation_property) {
  - drm_object_property_set_value(plane-base,
  - dev-mode_config.rotation_property,
  - BIT(DRM_ROTATE_0));
  + drm_mode_plane_set_obj_prop(plane,
  + 
  dev-mode_config.rotation_property,
  + BIT(DRM_ROTATE_0));
}
}
 
  diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
  index 0375d75..31344bf 100644
  --- a/include/drm/drm_crtc.h
  +++ b/include/drm/drm_crtc.h
  @@ -1127,6 +1127,9 @@ 

[Intel-gfx] [PATCH] drm/i915/bdw: Don't write PDP in the legacy way when using LRCs

2014-08-15 Thread Thomas Daniel
From: Oscar Mateo oscar.ma...@intel.com

This is mostly for correctness so that we know we are running the LR
context correctly (this is, the PDPs are contained inside the context
object).

v2: Move the check to inside the enable PPGTT function. The switch
happens in two places: the legacy context switch (that we won't hit
when Execlists are enabled) and the PPGTT enable, which unfortunately
we need. This would look much nicer if the ppgtt-enable was part of
the ring init, where it logically belongs.

v3: Move the check to the start of the enable PPGTT function.  None
of the legacy PPGTT enabling is required when using LRCs as the
PPGTT is enabled in the context descriptor and the PDPs are written
in the LRC.

v4: Clarify comment based on review feedback.

Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5188936..2966b53 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -843,6 +843,12 @@ static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
struct intel_engine_cs *ring;
int j, ret;
 
+   /* In the case of execlists, PPGTT is enabled by the context descriptor
+* and the PDPs are contained within the context itself.  We don't
+* need to do anything here. */
+   if (i915.enable_execlists)
+   return 0;
+
for_each_ring(ring, dev_priv, j) {
I915_WRITE(RING_MODE_GEN7(ring),
   _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH 2/5] drm/i915: Do not access stolen memory directly by the CPU, even for error capture

2014-08-15 Thread Mika Kuoppala
Chris Wilson ch...@chris-wilson.co.uk writes:

 For stolen pages, since it is verboten to access them directly on many
 architectures, we have to read them through the GTT aperture. If they
 are not accessible through the aperture, then we have to abort.

 This was complicated by

 commit 8b6124a633d8095b0c8364f585edff9c59568a96
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Thu Jan 30 14:38:16 2014 +

 drm/i915: Don't access snooped pages through the GTT (even for error 
 capture)

 and the desire to use stolen memory for ringbuffers, contexts and
 batches in the future.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_gpu_error.c | 50 
 ++-
  1 file changed, 31 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 35e70d5..6d280c07 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -561,10 +561,11 @@ static struct drm_i915_error_object *
  i915_error_object_create_sized(struct drm_i915_private *dev_priv,
  struct drm_i915_gem_object *src,
  struct i915_address_space *vm,
 -const int num_pages)
 +int num_pages)
  {
   struct drm_i915_error_object *dst;
 - int i;
 + bool use_ggtt;
 + int i = 0;
   u32 reloc_offset;
  
   if (src == NULL || src-pages == NULL)
 @@ -574,8 +575,32 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
   if (dst == NULL)
   return NULL;
  
 - reloc_offset = dst-gtt_offset = i915_gem_obj_offset(src, vm);
 - for (i = 0; i  num_pages; i++) {
 + dst-gtt_offset = i915_gem_obj_offset(src, vm);
 +
 + reloc_offset = dst-gtt_offset;
 + use_ggtt = (src-cache_level == I915_CACHE_NONE 
 
Take this cache level check out so that we end up doing the
snoopable check for non stolen objects too?

-Mika

 + i915_is_ggtt(vm) 
 + src-has_global_gtt_mapping 
 + reloc_offset + num_pages * PAGE_SIZE = 
 dev_priv-gtt.mappable_end);
 +
 + /* Cannot access stolen address directly, try to use the aperture */
 + if (src-stolen) {
 + use_ggtt = true;
 +
 + if (!src-has_global_gtt_mapping)
 + goto unwind;
 +
 + reloc_offset = i915_gem_obj_ggtt_offset(src);
 + if (reloc_offset + num_pages * PAGE_SIZE  
 dev_priv-gtt.mappable_end)
 + goto unwind;
 + }
 +
 + /* Cannot access snooped pages through the aperture */
 + if (use_ggtt  src-cache_level != I915_CACHE_NONE  
 !HAS_LLC(dev_priv-dev))
 + goto unwind;
 +
 + dst-page_count = num_pages;
 + while (num_pages--) {
   unsigned long flags;
   void *d;
  
 @@ -584,10 +609,7 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
   goto unwind;
  
   local_irq_save(flags);
 - if (src-cache_level == I915_CACHE_NONE 
 - reloc_offset  dev_priv-gtt.mappable_end 
 - src-has_global_gtt_mapping 
 - i915_is_ggtt(vm)) {
 + if (use_ggtt) {
   void __iomem *s;
  
   /* Simply ignore tiling or any overlapping fence.
 @@ -599,14 +621,6 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
reloc_offset);
   memcpy_fromio(d, s, PAGE_SIZE);
   io_mapping_unmap_atomic(s);
 - } else if (src-stolen) {
 - unsigned long offset;
 -
 - offset = dev_priv-mm.stolen_base;
 - offset += src-stolen-start;
 - offset += i  PAGE_SHIFT;
 -
 - memcpy_fromio(d, (void __iomem *) offset, PAGE_SIZE);
   } else {
   struct page *page;
   void *s;
 @@ -623,11 +637,9 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
   }
   local_irq_restore(flags);
  
 - dst-pages[i] = d;
 -
 + dst-pages[i++] = d;
   reloc_offset += PAGE_SIZE;
   }
 - dst-page_count = num_pages;
  
   return dst;
  
 -- 
 1.9.1

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


Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix HPD IRQ reenable work cancelation

2014-08-15 Thread Imre Deak
On Fri, 2014-08-15 at 12:48 +0300, Jani Nikula wrote:
 On Fri, 15 Aug 2014, Imre Deak imre.d...@intel.com wrote:
  On Wed, 2014-08-13 at 19:33 +0300, Ville Syrjälä wrote:
  The series seems fine to me.
  
  Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
  for the rest as well.
 
  Thanks, I assume it's for v2. I'd say this is for -fixes. The problem
  existed even in 3.16, but only the MST support made it apparent with the
  extra HPD signaling and DP AUX activity during suspend.
 
 The series no longer applies on any branch. Would you mind respinning on
 -fixes please?

Ok. I just noticed that it depends on the following two patches that are
only in -nightly not in -fixes:

drm/i915: Introduce a for_each_intel_encoder() macro
drm/i915: lock around link status and link training.

Are you going to apply these? The second one is definitely needed in
-fixes imo.

--Imre


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/dp: Backlight PWM enable before BL Enable assert

2014-08-15 Thread Jani Nikula
On Fri, 15 Aug 2014, clinton.a.tay...@intel.com wrote:
 From: Clint Taylor clinton.a.tay...@intel.com

 Backlight on delay uses PWM enable time to seperate PWM to
 backlight enable assert. Previous time difference used timing
 from VDD enable which occur several seconds before resulting
 in PWM starting 5ms after backlight enable. Changes to backlight
  duty cycle take affect at the end of the current PWM cycle.
 Measured time for the PWM cycle is 5ms. 5 additional ms must be
 added to the backlight_on_delay to get correct VBT timing of
 PWM to backlight enable assert.

The patch seems sane, but I'd like you to rebase this on top of patch
1/4 of my backlight series [1] so I can queue them both to
drm-intel-fixes. Then we'll have fewer conflicts with the rest of the
backlight series going forward.

Hint hint, I'd also appreciate review of my backlight series. ;)

BR,
Jani.


[1] 
http://mid.gmane.org/e7a47b50ed0f25cafdc26711fc09561ea8af3b81.1407849872.git.jani.nik...@intel.com



 Change-Id: I484999a597fd84dacf4cf99a168ec9ba4bb6ff11
 Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c  | 6 --
  drivers/gpu/drm/i915/intel_drv.h | 1 +
  2 files changed, 5 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index e5ada4f..c59ccdb 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1141,7 +1141,7 @@ static void wait_panel_power_cycle(struct intel_dp 
 *intel_dp)
  
  static void wait_backlight_on(struct intel_dp *intel_dp)
  {
 - wait_remaining_ms_from_jiffies(intel_dp-last_power_on,
 + wait_remaining_ms_from_jiffies(intel_dp-last_backlight_on,
  intel_dp-backlight_on_delay);
  }
  
 @@ -1398,6 +1398,7 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
   DRM_DEBUG_KMS(\n);
  
   intel_panel_enable_backlight(intel_dp-attached_connector);
 + intel_dp-last_backlight_on = jiffies;
  
   /*
* If we enable the backlight right away following a panel power
 @@ -4243,9 +4244,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device 
 *dev,
   assign_final(t11_t12);
  #undef assign_final
  
 +#define PWM_CYCLE_DELAY 5
  #define get_delay(field) (DIV_ROUND_UP(final.field, 10))
   intel_dp-panel_power_up_delay = get_delay(t1_t3);
 - intel_dp-backlight_on_delay = get_delay(t8);
 + intel_dp-backlight_on_delay = get_delay(t8) + PWM_CYCLE_DELAY;
   intel_dp-backlight_off_delay = get_delay(t9);
   intel_dp-panel_power_down_delay = get_delay(t10);
   intel_dp-panel_power_cycle_delay = get_delay(t11_t12);
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 3abc915..ad6fcc1 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -556,6 +556,7 @@ struct intel_dp {
   bool want_panel_vdd;
   unsigned long last_power_cycle;
   unsigned long last_power_on;
 + unsigned long last_backlight_on;
   unsigned long last_backlight_off;
  
   struct notifier_block edp_notifier;
 -- 
 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 35/43] drm/i915/bdw: Make sure error capture keeps working with Execlists

2014-08-15 Thread Daniel Vetter
On Thu, Jul 24, 2014 at 05:04:43PM +0100, Thomas Daniel wrote:
 From: Oscar Mateo oscar.ma...@intel.com
 
 Since the ringbuffer does not belong per engine anymore, we have to
 make sure that we are always recording the correct ringbuffer.
 
 TODO: This is only a small fix to keep basic error capture working, but
 we need to add more information for it to be useful (e.g. dump the
 context being executed).
 
 v2: Reorder how the ringbuffer is chosen to clarify the change and
 rename the variable, both changes suggested by Chris Wilson. Also,
 add the TODO comment to the code, as suggested by Daniel.
 
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com

There's a bit too much stuff in-flight to fix up error capture for ppgtt.
I think it's better to stall this patch here until that work is completed.
Please coordinate with Mika here.
-Daniel

 ---
  drivers/gpu/drm/i915/i915_gpu_error.c |   22 ++
  1 file changed, 18 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 45b6191..1e38576 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -874,9 +874,6 @@ static void i915_record_ring_state(struct drm_device *dev,
   ering-hws = I915_READ(mmio);
   }
  
 - ering-cpu_ring_head = ring-buffer-head;
 - ering-cpu_ring_tail = ring-buffer-tail;
 -
   ering-hangcheck_score = ring-hangcheck.score;
   ering-hangcheck_action = ring-hangcheck.action;
  
 @@ -936,6 +933,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
  
   for (i = 0; i  I915_NUM_RINGS; i++) {
   struct intel_engine_cs *ring = dev_priv-ring[i];
 + struct intel_ringbuffer *rbuf;
  
   error-ring[i].pid = -1;
  
 @@ -979,8 +977,24 @@ static void i915_gem_record_rings(struct drm_device *dev,
   }
   }
  
 + if (i915.enable_execlists) {
 + /* TODO: This is only a small fix to keep basic error
 +  * capture working, but we need to add more information
 +  * for it to be useful (e.g. dump the context being
 +  * executed).
 +  */
 + if (request)
 + rbuf = request-ctx-engine[ring-id].ringbuf;
 + else
 + rbuf = 
 ring-default_context-engine[ring-id].ringbuf;
 + } else
 + rbuf = ring-buffer;
 +
 + error-ring[i].cpu_ring_head = rbuf-head;
 + error-ring[i].cpu_ring_tail = rbuf-tail;
 +
   error-ring[i].ringbuffer =
 - i915_error_ggtt_object_create(dev_priv, 
 ring-buffer-obj);
 + i915_error_ggtt_object_create(dev_priv, rbuf-obj);
  
   if (ring-status_page.obj)
   error-ring[i].hws_page =
 -- 
 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 40/43] drm/i915/bdw: Document Logical Rings, LR contexts and Execlists

2014-08-15 Thread Daniel Vetter
On Thu, Jul 24, 2014 at 05:04:48PM +0100, Thomas Daniel wrote:
 +/**
 + * intel_lr_context_render_state_init() - render state init for Execlists
 + * @ring: Engine Command Streamer.
 + * @ctx: Context to initialize.
 + *
 + * A.K.A. null-context, A.K.A. golden-context. In a word, the render engine
 + * contexts require to always have a valid 3d pipeline state. As this is
 + * achieved with the submission of a batchbuffer, we require an alternative
 + * entry point to the legacy ringbuffer submission one 
 (i915_gem_render_state_init).
 + *
 + * Return: non-zero if the initialization failed.
 + */

I've dropped this hunk here since that part isn't merged yet.
-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 42/43] drm/i915/bdw: Pin the context backing objects to GGTT on-demand

2014-08-15 Thread Daniel Vetter
On Thu, Jul 24, 2014 at 05:04:50PM +0100, Thomas Daniel wrote:
 From: Oscar Mateo oscar.ma...@intel.com
 
 Up until now, we have pinned every logical ring context backing object
 during creation, and left it pinned until destruction. This made my life
 easier, but it's a harmful thing to do, because we cause fragmentation
 of the GGTT (and, eventually, we would run out of space).
 
 This patch makes the pinning on-demand: the backing objects of the two
 contexts that are written to the ELSP are pinned right before submission
 and unpinned once the hardware is done with them. The only context that
 is still pinned regardless is the global default one, so that the HWS can
 still be accessed in the same way (ring-status_page).
 
 v2: In the early version of this patch, we were pinning the context as
 we put it into the ELSP: on the one hand, this is very efficient because
 only a maximum two contexts are pinned at any given time, but on the other
 hand, we cannot really pin in interrupt time :(
 
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c |   11 +++--
  drivers/gpu/drm/i915/i915_drv.h |1 +
  drivers/gpu/drm/i915/i915_gem.c |   44 
 ---
  drivers/gpu/drm/i915/intel_lrc.c|   42 -
  drivers/gpu/drm/i915/intel_lrc.h|2 ++
  5 files changed, 73 insertions(+), 27 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 968c3c0..84531cc 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1721,10 +1721,15 @@ static int i915_dump_lrc(struct seq_file *m, void 
 *unused)
   continue;
  
   if (ctx_obj) {
 - struct page *page = 
 i915_gem_object_get_page(ctx_obj, 1);
 - uint32_t *reg_state = kmap_atomic(page);
 + struct page *page ;
 + uint32_t *reg_state;
   int j;
  
 + i915_gem_obj_ggtt_pin(ctx_obj, 
 GEN8_LR_CONTEXT_ALIGN, 0);

This just needs a get/put_pages, no pinning required.

 +
 + page = i915_gem_object_get_page(ctx_obj, 1);
 + reg_state = kmap_atomic(page);
 +
   seq_printf(m, CONTEXT: %s %u\n, ring-name,
   
 intel_execlists_ctx_id(ctx_obj));
  
 @@ -1736,6 +1741,8 @@ static int i915_dump_lrc(struct seq_file *m, void 
 *unused)
   }
   kunmap_atomic(reg_state);
  
 + i915_gem_object_ggtt_unpin(ctx_obj);
 +
   seq_putc(m, '\n');
   }
   }
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 1ce51d6..70466af 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -628,6 +628,7 @@ struct intel_context {
   struct {
   struct drm_i915_gem_object *state;
   struct intel_ringbuffer *ringbuf;
 + atomic_t unpin_count;

No need to reinvent wheels for this. We should be able to do exactly what
we've done for the legacy ctx objects, namely:
- Always pin the default system context so that we can always switch to
  that.
- Shovel all context releated objects through the active queue and obj
  management. This might depend upon the reworked exec list item.
- In the shrinker have a last-ditch effort to switch to the default
  context in case we run out of space.
- igt testcase. For legacy rings this code here resulted in some _really_
  expensive bugs and regressions.

I'll do another jira for this.

Otherwise I think I've pulled pretty much everything in except those
places where I think directly reworking the patches makes more sense. And
we should have JIRA tasks for all the outstanding work (plus ofc getting
ppgtt ready since execlists requires that).

If there's a patch I've missed or where people think it makes more sense
to merge the wip version now, please pipe up.

Thanks, 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 09/16] drm/i915: Kill useless ns2501_dump_regs

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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



Looks perfectly fine to me.

Signed-off-by: Thomas Richter rich...@rus.uni-stuttgart.de


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

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 74f2af7..85030d4 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -479,22 +479,6 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool 
enable)
}
  }

-static void ns2501_dump_regs(struct intel_dvo_device *dvo)
-{
-   uint8_t val;
-
-   ns2501_readb(dvo, NS2501_FREQ_LO, val);
-   DRM_DEBUG_KMS(NS2501_FREQ_LO: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_FREQ_HI, val);
-   DRM_DEBUG_KMS(NS2501_FREQ_HI: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REG8, val);
-   DRM_DEBUG_KMS(NS2501_REG8: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REG9, val);
-   DRM_DEBUG_KMS(NS2501_REG9: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REGC, val);
-   DRM_DEBUG_KMS(NS2501_REGC: 0x%02x\n, val);
-}
-
  static void ns2501_destroy(struct intel_dvo_device *dvo)
  {
struct ns2501_priv *ns = dvo-dev_priv;
@@ -512,6 +496,5 @@ struct intel_dvo_dev_ops ns2501_ops = {
.mode_set = ns2501_mode_set,
.dpms = ns2501_dpms,
.get_hw_state = ns2501_get_hw_state,
-   .dump_regs = ns2501_dump_regs,
.destroy = ns2501_destroy,
  };



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


Re: [Intel-gfx] [PATCH 10/16] drm/i915: Rewrite ns2501 driver a bit

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

Try to use the same programming sequence as used by the IEGD driver.

Also shovel the magic register values into a big static const array.

The register values are actually the based on what the BIOS programs
on the Fujitsu-Siemens Lifebook S6010. IEGD seemed to have hardcoded
register values (which also enabled the scaler for 1024x768 mode).
However those didn't actually work so well on the S6010. Possibly the
pipe timings that got used didn't match the ns2501 configuration.


Looks fine. Hard to say what these values actually mean, I tried to find 
some reasonable algorithm to derive them, but had little success. The 
current set of values is working for the S6010, though probably not for 
other devices using the same DVO but having a different display size.


Signed-off-by: Thomas Richter rich...@rus.uni-stuttgart.de





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

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 85030d4..b278571 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -60,16 +60,291 @@

  #define NS2501_REGC 0x0c

+enum {
+   MODE_640x480,
+   MODE_800x600,
+   MODE_1024x768,
+};
+
+struct ns2501_reg {
+uint8_t offset;
+uint8_t value;
+};
+
+/*
+ * Magic values based on what the BIOS on
+ * Fujitsu-Siemens Lifebook S6010 programs (1024x768 panel).
+ */
+static const struct ns2501_reg regs_1024x768[][86] = {
+   [MODE_640x480] = {
+   [0] = { .offset = 0x0a, .value = 0x81, },
+   [1] = { .offset = 0x18, .value = 0x07, },
+   [2] = { .offset = 0x19, .value = 0x00, },
+   [3] = { .offset = 0x1a, .value = 0x00, },
+   [4] = { .offset = 0x1b, .value = 0x11, },
+   [5] = { .offset = 0x1c, .value = 0x54, },
+   [6] = { .offset = 0x1d, .value = 0x03, },
+   [7] = { .offset = 0x1e, .value = 0x02, },
+   [8] = { .offset = 0xf3, .value = 0x90, },
+   [9] = { .offset = 0xf9, .value = 0x00, },
+   [10] = { .offset = 0xc1, .value = 0x90, },
+   [11] = { .offset = 0xc2, .value = 0x00, },
+   [12] = { .offset = 0xc3, .value = 0x0f, },
+   [13] = { .offset = 0xc4, .value = 0x03, },
+   [14] = { .offset = 0xc5, .value = 0x16, },
+   [15] = { .offset = 0xc6, .value = 0x00, },
+   [16] = { .offset = 0xc7, .value = 0x02, },
+   [17] = { .offset = 0xc8, .value = 0x02, },
+   [18] = { .offset = 0xf4, .value = 0x00, },
+   [19] = { .offset = 0x80, .value = 0xff, },
+   [20] = { .offset = 0x81, .value = 0x07, },
+   [21] = { .offset = 0x82, .value = 0x3d, },
+   [22] = { .offset = 0x83, .value = 0x05, },
+   [23] = { .offset = 0x94, .value = 0x00, },
+   [24] = { .offset = 0x95, .value = 0x00, },
+   [25] = { .offset = 0x96, .value = 0x05, },
+   [26] = { .offset = 0x97, .value = 0x00, },
+   [27] = { .offset = 0x9a, .value = 0x88, },
+   [28] = { .offset = 0x9b, .value = 0x00, },
+   [29] = { .offset = 0x98, .value = 0x00, },
+   [30] = { .offset = 0x99, .value = 0x00, },
+   [31] = { .offset = 0xf7, .value = 0x88, },
+   [32] = { .offset = 0xf8, .value = 0x0a, },
+   [33] = { .offset = 0x9c, .value = 0x24, },
+   [34] = { .offset = 0x9d, .value = 0x00, },
+   [35] = { .offset = 0x9e, .value = 0x25, },
+   [36] = { .offset = 0x9f, .value = 0x03, },
+   [37] = { .offset = 0xa0, .value = 0x28, },
+   [38] = { .offset = 0xa1, .value = 0x01, },
+   [39] = { .offset = 0xa2, .value = 0x28, },
+   [40] = { .offset = 0xa3, .value = 0x05, },
+   [41] = { .offset = 0xb6, .value = 0x09, },
+   [42] = { .offset = 0xb8, .value = 0x00, },
+   [43] = { .offset = 0xb9, .value = 0xa0, },
+   [44] = { .offset = 0xba, .value = 0x00, },
+   [45] = { .offset = 0xbb, .value = 0x20, },
+   [46] = { .offset = 0x10, .value = 0x00, },
+   [47] = { .offset = 0x11, .value = 0xa0, },
+   [48] = { .offset = 0x12, .value = 0x02, },
+   [49] = { .offset = 0x20, .value = 0x00, },
+   [50] = { .offset = 0x22, .value = 0x00, },
+   [51] = { .offset = 0x23, .value = 0x00, },
+   [52] = { .offset = 0x24, .value = 0x00, },
+   [53] = { .offset = 0x25, .value = 0x00, },
+   [54] = { .offset = 0x8c, .value = 0x10, },
+   

Re: [Intel-gfx] [PATCH 11/16] drm/i915: Init important ns2501 registers

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

In my earlier rewrite I missed a few important registers. Thomas Richter
noticed that they're needed to make his machine resume correctly.

Looks like IEGD does a one time init of these three registers. We don't
have a good one time init place in the ns2501 driver, so let's just
stick them into the .mode_set() hook and see if that helps things along.


Looks good to me.

Signed-off-by: Thomas Richter rich...@rus.uni-stuttgart.de



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

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index b278571..345235b 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
},
  };

+static const struct ns2501_reg regs_init[] = {
+   [0] = { .offset = 0x35, .value = 0xff, },
+   [1] = { .offset = 0x34, .value = 0x00, },
+   [2] = { .offset = 0x08, .value = 0x30, },
+};
+
  struct ns2501_priv {
bool quiet;
const struct ns2501_reg *regs;
@@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
else
return;

+   /* Hopefully doing it every time won't hurt... */
+   for (i = 0; i  ARRAY_SIZE(regs_init); i++)
+   ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
+
ns-regs = regs_1024x768[mode_idx];

for (i = 0; i  84; i++)



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


Re: [Intel-gfx] [PATCH 12/16] drm/i915: Check pixel clock in ns2501 mode_valid hook

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

The vbt on my Fujitsu-Siemens Lifebook S6010 provides two 800x600 modes,
60Hz and 56Hz. The magic register values we have correspond to the 60Hz
mode, and as I don't know how one would trick the VGA BIOS to set up
the 56Hz mode we can't get the magic values for the orther mode. So
when checking whether a mode is valid also check the pixel clock so that
we filter out the 56Hz variant.


The 56Hz mode works here as does the 60Hz mode, though it is really 
better to have only one 800x600 mode. Looks fine.


Signed-off-by: Thomas Richter rich...@rus.uni-stuttgart.de



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

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 345235b..4416304 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -521,9 +521,9 @@ static enum drm_mode_status ns2501_mode_valid(struct 
intel_dvo_device *dvo,
 * of the panel in here so we could always accept it
 * by disabling the scaler.
 */
-   if ((mode-hdisplay == 800  mode-vdisplay == 600) ||
-   (mode-hdisplay == 640  mode-vdisplay == 480) ||
-   (mode-hdisplay == 1024  mode-vdisplay == 768)) {
+   if ((mode-hdisplay == 640  mode-vdisplay == 480  mode-clock == 
25175) ||
+   (mode-hdisplay == 800  mode-vdisplay == 600  mode-clock == 
4) ||
+   (mode-hdisplay == 1024  mode-vdisplay == 768  mode-clock == 
65000)) {
return MODE_OK;
} else {
return MODE_ONE_SIZE;   /* Is this a reasonable error? */



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


Re: [Intel-gfx] [PATCH 01/16] drm/i915: Fix gen2 planes B and C max watermark value

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:21, ville.syrj...@linux.intel.com wrote:

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

The max watermark value for gen2 planes B and C is 0x1f, instead of
the 0x3f that plane A uses.

Also check against the max even if the pipe is disabled since the
FIFO size exceeds the plane B and C max watermark value.



Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 12f4e14..f696b7f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -985,13 +985,20 @@ static const struct intel_watermark_params i915_wm_info = 
{
.guard_size = 2,
.cacheline_size = I915_FIFO_LINE_SIZE,
  };
-static const struct intel_watermark_params i830_wm_info = {
+static const struct intel_watermark_params i830_a_wm_info = {
.fifo_size = I855GM_FIFO_SIZE,
.max_wm = I915_MAX_WM,
.default_wm = 1,
.guard_size = 2,
.cacheline_size = I830_FIFO_LINE_SIZE,
  };
+static const struct intel_watermark_params i830_bc_wm_info = {
+   .fifo_size = I855GM_FIFO_SIZE,
+   .max_wm = I915_MAX_WM/2,
+   .default_wm = 1,
+   .guard_size = 2,
+   .cacheline_size = I830_FIFO_LINE_SIZE,
+};
  static const struct intel_watermark_params i845_wm_info = {
.fifo_size = I830_FIFO_SIZE,
.max_wm = I915_MAX_WM,
@@ -1673,7 +1680,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
else if (!IS_GEN2(dev))
wm_info = i915_wm_info;
else
-   wm_info = i830_wm_info;
+   wm_info = i830_a_wm_info;

fifo_size = dev_priv-display.get_fifo_size(dev, 0);
crtc = intel_get_crtc_for_plane(dev, 0);
@@ -1688,8 +1695,14 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
   wm_info, fifo_size, cpp,
   latency_ns);
enabled = crtc;
-   } else
+   } else {
planea_wm = fifo_size - wm_info-guard_size;
+   if (planea_wm  (long)wm_info-max_wm)
+   planea_wm = wm_info-max_wm;
+   }
+
+   if (IS_GEN2(dev))
+   wm_info = i830_bc_wm_info;

fifo_size = dev_priv-display.get_fifo_size(dev, 1);
crtc = intel_get_crtc_for_plane(dev, 1);
@@ -1707,8 +1720,11 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
enabled = crtc;
else
enabled = NULL;
-   } else
+   } else {
planeb_wm = fifo_size - wm_info-guard_size;
+   if (planeb_wm  (long)wm_info-max_wm)
+   planeb_wm = wm_info-max_wm;
+   }

DRM_DEBUG_KMS(FIFO watermarks - A: %d, B: %d\n, planea_wm, planeb_wm);




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


Re: [Intel-gfx] [PATCH 04/16] drm/i915: Pass intel_crtc to intel_disable_pipe() and intel_wait_for_pipe_off()

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:21, ville.syrj...@linux.intel.com wrote:

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

Just pass the intel_crtc around instead of dev_priv+pipe.

Also make intel_wait_for_pipe_off() static since it's only used in
intel_display.c.

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_display.c | 37 +---
  drivers/gpu/drm/i915/intel_drv.h |  1 -
  2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3813526..e7175ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -913,8 +913,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum 
pipe pipe)

  /*
   * intel_wait_for_pipe_off - wait for pipe to turn off
- * @dev: drm device
- * @pipe: pipe to wait for
+ * @crtc: crtc whose pipe to wait for
   *
   * After disabling a pipe, we can't wait for vblank in the usual way,
   * spinning on the vblank interrupt status bit, since we won't actually
@@ -928,11 +927,12 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum 
pipe pipe)
   *   ends up stopping at the start of the next frame).
   *
   */
-void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
+static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
  {
+   struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
-   enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
- pipe);
+   enum transcoder cpu_transcoder = crtc-config.cpu_transcoder;
+   enum pipe pipe = crtc-pipe;

if (INTEL_INFO(dev)-gen = 4) {
int reg = PIPECONF(cpu_transcoder);
@@ -1981,21 +1981,19 @@ static void intel_enable_pipe(struct intel_crtc *crtc)

  /**
   * intel_disable_pipe - disable a pipe, asserting requirements
- * @dev_priv: i915 private structure
- * @pipe: pipe to disable
- *
- * Disable @pipe, making sure that various hardware specific requirements
- * are met, if applicable, e.g. plane disabled, panel fitter off, etc.
+ * @crtc: crtc whose pipes is to be disabled
   *
- * @pipe should be %PIPE_A or %PIPE_B.
+ * Disable the pipe of @crtc, making sure that various hardware
+ * specific requirements are met, if applicable, e.g. plane
+ * disabled, panel fitter off, etc.
   *
   * Will wait until the pipe has shut down before returning.
   */
-static void intel_disable_pipe(struct drm_i915_private *dev_priv,
-  enum pipe pipe)
+static void intel_disable_pipe(struct intel_crtc *crtc)
  {
-   enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
- pipe);
+   struct drm_i915_private *dev_priv = crtc-base.dev-dev_private;
+   enum transcoder cpu_transcoder = crtc-config.cpu_transcoder;
+   enum pipe pipe = crtc-pipe;
int reg;
u32 val;

@@ -2017,7 +2015,7 @@ static void intel_disable_pipe(struct drm_i915_private 
*dev_priv,
return;

I915_WRITE(reg, val  ~PIPECONF_ENABLE);
-   intel_wait_for_pipe_off(dev_priv-dev, pipe);
+   intel_wait_for_pipe_off(crtc);
  }

  /*
@@ -4115,7 +4113,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
if (intel_crtc-config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, pipe, false);

-   intel_disable_pipe(dev_priv, pipe);
+   intel_disable_pipe(intel_crtc);

if (intel_crtc-config.dp_encoder_is_mst)
intel_ddi_set_vc_payload_alloc(crtc, false);
@@ -4167,7 +4165,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
-   int pipe = intel_crtc-pipe;
enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder;

if (!intel_crtc-active)
@@ -4182,7 +4179,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)

if (intel_crtc-config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, false);
-   intel_disable_pipe(dev_priv, pipe);
+   intel_disable_pipe(intel_crtc);

intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);

@@ -4769,7 +4766,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 */
intel_wait_for_vblank(dev, pipe);

-   intel_disable_pipe(dev_priv, pipe);
+   intel_disable_pipe(intel_crtc);

i9xx_pfit_disable(intel_crtc);

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3abc915..6c8303e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ 

Re: [Intel-gfx] [PATCH v3 05/16] drm/i915: Disable double wide even when leaving the pipe on

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:21, ville.syrj...@linux.intel.com wrote:

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

Disable double wide even if the pipe quirk compels us to leave the
pipe running. Double wide has certain implications for the plane
assignments so best keep it off.

Also helps resuming from S3 on the Fujitsu-Siemens Lifebook S6010
when double wide was enabled prior to suspend.

We do leave the pixel clock ticking at the original rate which would
require double wide to be enabled. But since the planes are all disabled
I'm hoping that the overly fast clock won't cause any problems. Seems
to be fine so far.

v2: Disable double wide also when turning the pipe off
v3: Reorder wrt. force pipe B quirk

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_display.c | 20 ++--
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e7175ce..3eeb5ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2005,17 +2005,25 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
assert_cursor_disabled(dev_priv, pipe);
assert_sprites_disabled(dev_priv, pipe);

-   /* Don't disable pipe A or pipe A PLLs if needed */
-   if (pipe == PIPE_A  (dev_priv-quirks  QUIRK_PIPEA_FORCE))
-   return;
-
reg = PIPECONF(cpu_transcoder);
val = I915_READ(reg);
if ((val  PIPECONF_ENABLE) == 0)
return;

-   I915_WRITE(reg, val  ~PIPECONF_ENABLE);
-   intel_wait_for_pipe_off(crtc);
+   /*
+* Double wide has implications for planes
+* so best keep it disabled when not needed.
+*/
+   if (crtc-config.double_wide)
+   val = ~PIPECONF_DOUBLE_WIDE;
+
+   /* Don't disable pipe or pipe PLLs if needed */
+   if (!(pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE))
+   val = ~PIPECONF_ENABLE;
+
+   I915_WRITE(reg, val);
+   if ((val  PIPECONF_ENABLE) == 0)
+   intel_wait_for_pipe_off(crtc);
  }

  /*



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


Re: [Intel-gfx] [PATCH 06/16] drm/i915: ns2501 is on DVOB

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:21, ville.syrj...@linux.intel.com wrote:

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

On Fujitsu-Siememens S6010 the ns2501 chip is hooked up to DVOB instead
of DVOC.

FIXME: Maybe need to dig out the correct DVO port from VBT

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_dvo.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 56b47d2..d5ea393 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -85,7 +85,7 @@ static const struct intel_dvo_device intel_dvo_devices[] = {
{
.type = INTEL_DVO_CHIP_TMDS,
.name = ns2501,
-   .dvo_reg = DVOC,
+   .dvo_reg = DVOB,
.slave_addr = NS2501_ADDR,
.dev_ops = ns2501_ops,
 }



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


Re: [Intel-gfx] [PATCH 08/16] drm/i915: Don't call DVO mode_set hook on DPMS changes

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

Calling the mode_set hook on DPMS changes doesn't seem to be necessary
for ns2501. Just drop it.

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_dvo.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 4f115c1..e40e3df 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -227,10 +227,6 @@ static void intel_dvo_dpms(struct drm_connector 
*connector, int mode)

intel_crtc_update_dpms(crtc);

-   intel_dvo-dev.dev_ops-mode_set(intel_dvo-dev,
-config-requested_mode,
-config-adjusted_mode);
-
intel_dvo-dev.dev_ops-dpms(intel_dvo-dev, true);
} else {
intel_dvo-dev.dev_ops-dpms(intel_dvo-dev, false);



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


Re: [Intel-gfx] [PATCH 07/16] drm/i915: Enable DVO between mode_set and dpms hooks

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:21, ville.syrj...@linux.intel.com wrote:

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

To more closely match the IEGD ns2501 driver behaviour, call the
mode_set hook while the DVO port is still disabled, then enable the DVO
port, and finally call the dpms hook.

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_dvo.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index d5ea393..4f115c1 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -185,12 +185,13 @@ static void intel_enable_dvo(struct intel_encoder 
*encoder)
u32 dvo_reg = intel_dvo-dev.dvo_reg;
u32 temp = I915_READ(dvo_reg);

-   I915_WRITE(dvo_reg, temp | DVO_ENABLE);
-   I915_READ(dvo_reg);
intel_dvo-dev.dev_ops-mode_set(intel_dvo-dev,
 crtc-config.requested_mode,
 crtc-config.adjusted_mode);

+   I915_WRITE(dvo_reg, temp | DVO_ENABLE);
+   I915_READ(dvo_reg);
+
intel_dvo-dev.dev_ops-dpms(intel_dvo-dev, true);
  }




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


Re: [Intel-gfx] [PATCH 09/16] drm/i915: Kill useless ns2501_dump_regs

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

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


Reviewed-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/dvo_ns2501.c | 17 -
  1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 74f2af7..85030d4 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -479,22 +479,6 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool 
enable)
}
  }

-static void ns2501_dump_regs(struct intel_dvo_device *dvo)
-{
-   uint8_t val;
-
-   ns2501_readb(dvo, NS2501_FREQ_LO, val);
-   DRM_DEBUG_KMS(NS2501_FREQ_LO: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_FREQ_HI, val);
-   DRM_DEBUG_KMS(NS2501_FREQ_HI: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REG8, val);
-   DRM_DEBUG_KMS(NS2501_REG8: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REG9, val);
-   DRM_DEBUG_KMS(NS2501_REG9: 0x%02x\n, val);
-   ns2501_readb(dvo, NS2501_REGC, val);
-   DRM_DEBUG_KMS(NS2501_REGC: 0x%02x\n, val);
-}
-
  static void ns2501_destroy(struct intel_dvo_device *dvo)
  {
struct ns2501_priv *ns = dvo-dev_priv;
@@ -512,6 +496,5 @@ struct intel_dvo_dev_ops ns2501_ops = {
.mode_set = ns2501_mode_set,
.dpms = ns2501_dpms,
.get_hw_state = ns2501_get_hw_state,
-   .dump_regs = ns2501_dump_regs,
.destroy = ns2501_destroy,
  };



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


Re: [Intel-gfx] [PATCH 10/16] drm/i915: Rewrite ns2501 driver a bit

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

Try to use the same programming sequence as used by the IEGD driver.

Also shovel the magic register values into a big static const array.

The register values are actually the based on what the BIOS programs
on the Fujitsu-Siemens Lifebook S6010. IEGD seemed to have hardcoded
register values (which also enabled the scaler for 1024x768 mode).
However those didn't actually work so well on the S6010. Possibly the
pipe timings that got used didn't match the ns2501 configuration.

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


Reviewed-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/dvo_ns2501.c | 529 +++---
  1 file changed, 325 insertions(+), 204 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 85030d4..b278571 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -60,16 +60,291 @@

  #define NS2501_REGC 0x0c

+enum {
+   MODE_640x480,
+   MODE_800x600,
+   MODE_1024x768,
+};
+
+struct ns2501_reg {
+uint8_t offset;
+uint8_t value;
+};
+
+/*
+ * Magic values based on what the BIOS on
+ * Fujitsu-Siemens Lifebook S6010 programs (1024x768 panel).
+ */
+static const struct ns2501_reg regs_1024x768[][86] = {
+   [MODE_640x480] = {
+   [0] = { .offset = 0x0a, .value = 0x81, },
+   [1] = { .offset = 0x18, .value = 0x07, },
+   [2] = { .offset = 0x19, .value = 0x00, },
+   [3] = { .offset = 0x1a, .value = 0x00, },
+   [4] = { .offset = 0x1b, .value = 0x11, },
+   [5] = { .offset = 0x1c, .value = 0x54, },
+   [6] = { .offset = 0x1d, .value = 0x03, },
+   [7] = { .offset = 0x1e, .value = 0x02, },
+   [8] = { .offset = 0xf3, .value = 0x90, },
+   [9] = { .offset = 0xf9, .value = 0x00, },
+   [10] = { .offset = 0xc1, .value = 0x90, },
+   [11] = { .offset = 0xc2, .value = 0x00, },
+   [12] = { .offset = 0xc3, .value = 0x0f, },
+   [13] = { .offset = 0xc4, .value = 0x03, },
+   [14] = { .offset = 0xc5, .value = 0x16, },
+   [15] = { .offset = 0xc6, .value = 0x00, },
+   [16] = { .offset = 0xc7, .value = 0x02, },
+   [17] = { .offset = 0xc8, .value = 0x02, },
+   [18] = { .offset = 0xf4, .value = 0x00, },
+   [19] = { .offset = 0x80, .value = 0xff, },
+   [20] = { .offset = 0x81, .value = 0x07, },
+   [21] = { .offset = 0x82, .value = 0x3d, },
+   [22] = { .offset = 0x83, .value = 0x05, },
+   [23] = { .offset = 0x94, .value = 0x00, },
+   [24] = { .offset = 0x95, .value = 0x00, },
+   [25] = { .offset = 0x96, .value = 0x05, },
+   [26] = { .offset = 0x97, .value = 0x00, },
+   [27] = { .offset = 0x9a, .value = 0x88, },
+   [28] = { .offset = 0x9b, .value = 0x00, },
+   [29] = { .offset = 0x98, .value = 0x00, },
+   [30] = { .offset = 0x99, .value = 0x00, },
+   [31] = { .offset = 0xf7, .value = 0x88, },
+   [32] = { .offset = 0xf8, .value = 0x0a, },
+   [33] = { .offset = 0x9c, .value = 0x24, },
+   [34] = { .offset = 0x9d, .value = 0x00, },
+   [35] = { .offset = 0x9e, .value = 0x25, },
+   [36] = { .offset = 0x9f, .value = 0x03, },
+   [37] = { .offset = 0xa0, .value = 0x28, },
+   [38] = { .offset = 0xa1, .value = 0x01, },
+   [39] = { .offset = 0xa2, .value = 0x28, },
+   [40] = { .offset = 0xa3, .value = 0x05, },
+   [41] = { .offset = 0xb6, .value = 0x09, },
+   [42] = { .offset = 0xb8, .value = 0x00, },
+   [43] = { .offset = 0xb9, .value = 0xa0, },
+   [44] = { .offset = 0xba, .value = 0x00, },
+   [45] = { .offset = 0xbb, .value = 0x20, },
+   [46] = { .offset = 0x10, .value = 0x00, },
+   [47] = { .offset = 0x11, .value = 0xa0, },
+   [48] = { .offset = 0x12, .value = 0x02, },
+   [49] = { .offset = 0x20, .value = 0x00, },
+   [50] = { .offset = 0x22, .value = 0x00, },
+   [51] = { .offset = 0x23, .value = 0x00, },
+   [52] = { .offset = 0x24, .value = 0x00, },
+   [53] = { .offset = 0x25, .value = 0x00, },
+   [54] = { .offset = 0x8c, .value = 0x10, },
+   [55] = { .offset = 0x8d, .value = 0x02, },
+   [56] = { .offset = 0x8e, .value = 0x10, },
+   [57] = { .offset = 0x8f, .value = 0x00, },
+   [58] = { .offset = 0x90, .value = 0xff, },
+   [59] = { .offset = 0x91, .value = 0x07, },
+   

Re: [Intel-gfx] [PATCH v2] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-08-15 Thread Mcaulay, Alistair
Hi Mika / Daniel,

below is the basic code path of a reset which has been changed by my patch:

i915_reset()
{

i915_gem_reset() - This used to call i915_gem_context_reset(), which 
has now been removed. 
.
i915_gem_init_hw()
.
i915_gem_context_enable() - This used to return during reset. 
Now it doesn't
.
for each ring, i915_switch_context(default)
do_switch();
.

.
}

 I am with Daniel on this one. I don't understand how can we throw everything 
in here away.
Did you maybe mean Ben?
Daniel, I thought you were happy with the implementation, and V2 fixed your 
last concern, could you please comment.

 We need to force hw to switch to a working context, after reset, so that our 
internal state tracking matches.
I believe this patch does that using i915_switch_context, rather than the hacky 
i915_gem_context_reset()

Alistair.

 -Original Message-
 From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com]
 Sent: Wednesday, August 06, 2014 5:25 PM
 To: Mcaulay, Alistair; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Rework GPU reset sequence to
 match driver load  thaw
 
 
 Hi,
 
 alistair.mcau...@intel.com writes:
 
  From: McAulay, Alistair alistair.mcau...@intel.com
 
  This patch is to address Daniels concerns over different code during reset:
 
  http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
 
  The reason for aiming as hard as possible to use the exact same code
  for driver load, gpu reset and runtime pm/system resume is that we've
  simply seen too many bugs due to slight variations and unintended
 omissions.
 
  Tested using igt drv_hangman.
 
  V2: Cleaner way of preventing check_wedge returning -EAGAIN
 
  Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.c |  6 +++
   drivers/gpu/drm/i915/i915_drv.h |  3 ++
   drivers/gpu/drm/i915/i915_gem.c |  6 +--
   drivers/gpu/drm/i915/i915_gem_context.c | 42 +
   drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
  +
   drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
   6 files changed, 23 insertions(+), 104 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_drv.c
  b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644
  --- a/drivers/gpu/drm/i915/i915_drv.c
  +++ b/drivers/gpu/drm/i915/i915_drv.c
  @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev)
  !dev_priv-ums.mm_suspended) {
  dev_priv-ums.mm_suspended = 0;
 
  +   /* Used to prevent gem_check_wedged returning -EAGAIN
 during gpu reset */
  +   dev_priv-gpu_error.reload_in_reset = true;
  +
  ret = i915_gem_init_hw(dev);
  +
  +   dev_priv-gpu_error.reload_in_reset = false;
  +
  mutex_unlock(dev-struct_mutex);
  if (ret) {
  DRM_ERROR(Failed hw init on reset %d\n, ret); diff
 --git
  a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
  index 991b663..116daff 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -1217,6 +1217,9 @@ struct i915_gpu_error {
 
  /* For missed irq/seqno simulation. */
  unsigned int test_irq_rings;
  +
  +   /* Used to prevent gem_check_wedged returning -EAGAIN during
 gpu reset   */
  +   bool reload_in_reset;
   };
 
   enum modeset_restore {
  diff --git a/drivers/gpu/drm/i915/i915_gem.c
  b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..14e1770 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error
 *error,
  if (i915_terminally_wedged(error))
  return -EIO;
 
  -   return -EAGAIN;
  +   /* Check if GPU Reset is in progress */
  +   if (!error-reload_in_reset)
  +   return -EAGAIN;
  }
 
  return 0;
  @@ -2590,8 +2592,6 @@ void i915_gem_reset(struct drm_device *dev)
  for_each_ring(ring, dev_priv, i)
  i915_gem_reset_ring_cleanup(dev_priv, ring);
 
  -   i915_gem_context_reset(dev);
  -
  i915_gem_restore_fences(dev);
   }
 
  diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
  b/drivers/gpu/drm/i915/i915_gem_context.c
  index de72a28..d96219f 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -372,42 +372,6 @@ err_destroy:
  return ERR_PTR(ret);
   }
 
  -void i915_gem_context_reset(struct drm_device *dev) -{
  -   struct drm_i915_private *dev_priv = dev-dev_private;
  -   int i;
  -
  -   /* Prevent the hardware from restoring the last context (which
 hung) on
  -* the next switch */
  -   for (i = 0; i  I915_NUM_RINGS; i++) {
  -   struct 

Re: [Intel-gfx] [PATCH 12/16] drm/i915: Check pixel clock in ns2501 mode_valid hook

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

The vbt on my Fujitsu-Siemens Lifebook S6010 provides two 800x600 modes,
60Hz and 56Hz. The magic register values we have correspond to the 60Hz
mode, and as I don't know how one would trick the VGA BIOS to set up
the 56Hz mode we can't get the magic values for the orther mode. So
when checking whether a mode is valid also check the pixel clock so that
we filter out the 56Hz variant.

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


Reviewed-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/dvo_ns2501.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 345235b..4416304 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -521,9 +521,9 @@ static enum drm_mode_status ns2501_mode_valid(struct 
intel_dvo_device *dvo,
 * of the panel in here so we could always accept it
 * by disabling the scaler.
 */
-   if ((mode-hdisplay == 800  mode-vdisplay == 600) ||
-   (mode-hdisplay == 640  mode-vdisplay == 480) ||
-   (mode-hdisplay == 1024  mode-vdisplay == 768)) {
+   if ((mode-hdisplay == 640  mode-vdisplay == 480  mode-clock == 
25175) ||
+   (mode-hdisplay == 800  mode-vdisplay == 600  mode-clock == 
4) ||
+   (mode-hdisplay == 1024  mode-vdisplay == 768  mode-clock == 
65000)) {
return MODE_OK;
} else {
return MODE_ONE_SIZE;   /* Is this a reasonable error? */



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


Re: [Intel-gfx] [PATCH 13/16] drm/i915: Fix DVO 2x clock enable on 830M

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

The spec says:
For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
(DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
both the DPLL A Control Register (06014h-06017h) and DPLL B Control
Register (06018h-0601Bh).

The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
just need a bit of special care to handle DPLL_DVO_2X_MODE.

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/i915_drv.h  |  3 +++
  drivers/gpu/drm/i915/intel_display.c | 47 +---
  2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 541fb6f..54895a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1561,6 +1561,9 @@ struct drm_i915_private {

u16 orig_clock;

+   /* used to control DVO 2x clock enable on 830M */
+   uint8_t dvo_pipes;
+
bool mchbar_need_disable;

struct intel_l3_parity l3_parity;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3eeb5ce..6462bcf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1548,6 +1548,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
  {
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   enum pipe pipe = crtc-pipe;
int reg = DPLL(crtc-pipe);
u32 dpll = crtc-config.dpll_hw_state.dpll;

@@ -1560,7 +1561,16 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
if (IS_MOBILE(dev)  !IS_I830(dev))
assert_panel_unlocked(dev_priv, crtc-pipe);

-   I915_WRITE(reg, dpll);
+   /* enable DVO 2x clock on both PLLs if necessary */
+   if (IS_I830(dev)) {
+   if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
+   dev_priv-dvo_pipes |= 1  pipe;
+
+   if (dev_priv-dvo_pipes) {
+   dpll |= DPLL_DVO_2X_MODE;
+   I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) | 
DPLL_DVO_2X_MODE);
+   }
+   }

/* Wait for the clocks to stabilize. */
POSTING_READ(reg);
@@ -1599,8 +1609,23 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
   *
   * Note!  This is for pre-ILK only.
   */
-static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
+static void i9xx_disable_pll(struct intel_crtc *crtc)
  {
+   struct drm_device *dev = crtc-base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   enum pipe pipe = crtc-pipe;
+
+   /* disable DVO 2x clock on both PLLs if necessary */
+   if (IS_I830(dev)) {
+   if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
+   dev_priv-dvo_pipes = ~(1  pipe);
+
+   if (!dev_priv-dvo_pipes) {
+   I915_WRITE(DPLL(pipe), I915_READ(DPLL(pipe))  
~DPLL_DVO_2X_MODE);
+   I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe))  
~DPLL_DVO_2X_MODE);
+   }
+   }
+
/* Don't disable pipe A or pipe A PLLs if needed */
if (pipe == PIPE_A  (dev_priv-quirks  QUIRK_PIPEA_FORCE))
return;
@@ -4788,7 +4813,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
else if (IS_VALLEYVIEW(dev))
vlv_disable_pll(dev_priv, pipe);
else
-   i9xx_disable_pll(dev_priv, pipe);
+   i9xx_disable_pll(intel_crtc);
}

if (!IS_GEN2(dev))
@@ -5792,7 +5817,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
dpll |= PLL_P2_DIVIDE_BY_4;
}

-   if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
+   if (!IS_I830(dev)  intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
dpll |= DPLL_DVO_2X_MODE;

if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS) 
@@ -6298,6 +6323,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
}
pipe_config-dpll_hw_state.dpll = I915_READ(DPLL(crtc-pipe));
if (!IS_VALLEYVIEW(dev)) {
+   if (IS_I830(dev))
+   pipe_config-dpll_hw_state.dpll = ~DPLL_DVO_2X_MODE;
+
pipe_config-dpll_hw_state.fp0 = I915_READ(FP0(crtc-pipe));
pipe_config-dpll_hw_state.fp1 = I915_READ(FP1(crtc-pipe));
} else {
@@ -13021,6 +13049,17 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
}
}

+   /* update dvo_pipes */
+   if (IS_I830(dev)) {
+   dev_priv-dvo_pipes = 0;
+
+   

Re: [Intel-gfx] [PATCH 14/16] Revert drm/i915: Nuke pipe A quirk on i830M

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

830 really does want the pipe A quirk. The planes and ports don't
react to any register writes unless the pipe currently attached
to them is running, so it's impossible to move them to the other
pipe unless both pipes are running.

Also it's documented that the DPLL must be enabled on both pipes
whenever it's needed.

This reverts commit ac6696d3236bd61503f89a1a99680fd7894d5d53.

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


Works on both the R31 and the S6010.

Reviewed-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/intel_display.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6462bcf..e1c0c0b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12503,6 +12503,9 @@ static struct intel_quirk intel_quirks[] = {
/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },

+   /* 830 needs to leave pipe A  dpll A up */
+   { 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
+
/* Lenovo U160 cannot use SSC on LVDS */
{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },




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


Re: [Intel-gfx] [PATCH v2 15/16] drm/i915: Add pipe B force quirk for 830M

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

830M has problems when some of the pipes are disabled. Namely if a
plane, DVO port etc. is currently assigned to a disabled pipe, it
can't moved to the other pipe until the current pipe is also enabled.
To keep things simple just leave both pipes running all the time.

Ideally I think should turn the pipes off if neither is active, and
when either becomes active we enable both. But that would reuquire
proper atomic modeset support, and probably a bit of extra care in
the order things get enabled.

v2: Reorder wrt. double wide handling changes

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


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/intel_display.c | 39 +---
  2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54895a6..b1ed71e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -706,6 +706,7 @@ enum intel_sbi_destination {
  #define QUIRK_LVDS_SSC_DISABLE (11)
  #define QUIRK_INVERT_BRIGHTNESS (12)
  #define QUIRK_BACKLIGHT_PRESENT (13)
+#define QUIRK_PIPEB_FORCE (14)

  struct intel_fbdev;
  struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e1c0c0b..92baf6f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1194,8 +1194,9 @@ void assert_pipe(struct drm_i915_private *dev_priv,
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
  pipe);

-   /* if we need the pipe A quirk it must be always on */
-   if (pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE)
+   /* if we need the pipe quirk it must be always on */
+   if ((pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE) ||
+   (pipe == PIPE_B  dev_priv-quirks  QUIRK_PIPEB_FORCE))
state = true;

if (!intel_display_power_enabled(dev_priv,
@@ -1626,8 +1627,9 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
}
}

-   /* Don't disable pipe A or pipe A PLLs if needed */
-   if (pipe == PIPE_A  (dev_priv-quirks  QUIRK_PIPEA_FORCE))
+   /* Don't disable pipe or pipe PLLs if needed */
+   if ((pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE) ||
+   (pipe == PIPE_B  dev_priv-quirks  QUIRK_PIPEB_FORCE))
return;

/* Make sure the pipe isn't still relying on us */
@@ -1995,8 +1997,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
reg = PIPECONF(cpu_transcoder);
val = I915_READ(reg);
if (val  PIPECONF_ENABLE) {
-   WARN_ON(!(pipe == PIPE_A 
- dev_priv-quirks  QUIRK_PIPEA_FORCE));
+   WARN_ON(!((pipe == PIPE_A  dev_priv-quirks  
QUIRK_PIPEA_FORCE) ||
+ (pipe == PIPE_B  dev_priv-quirks  
QUIRK_PIPEB_FORCE)));
return;
}

@@ -2043,7 +2045,8 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
val = ~PIPECONF_DOUBLE_WIDE;

/* Don't disable pipe or pipe PLLs if needed */
-   if (!(pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE))
+   if (!(pipe == PIPE_A  dev_priv-quirks  QUIRK_PIPEA_FORCE) 
+   !(pipe == PIPE_B  dev_priv-quirks  QUIRK_PIPEB_FORCE))
val = ~PIPECONF_ENABLE;

I915_WRITE(reg, val);
@@ -5968,9 +5971,9 @@ static void i9xx_set_pipeconf(struct intel_crtc 
*intel_crtc)

pipeconf = 0;

-   if (dev_priv-quirks  QUIRK_PIPEA_FORCE 
-   I915_READ(PIPECONF(intel_crtc-pipe))  PIPECONF_ENABLE)
-   pipeconf |= PIPECONF_ENABLE;
+   if ((intel_crtc-pipe == PIPE_A  dev_priv-quirks  
QUIRK_PIPEA_FORCE) ||
+   (intel_crtc-pipe == PIPE_B  dev_priv-quirks  
QUIRK_PIPEB_FORCE))
+   pipeconf |= I915_READ(PIPECONF(intel_crtc-pipe))  
PIPECONF_ENABLE;

if (intel_crtc-config.double_wide)
pipeconf |= PIPECONF_DOUBLE_WIDE;
@@ -10680,8 +10683,9 @@ check_crtc_state(struct drm_device *dev)
active = dev_priv-display.get_pipe_config(crtc,
   pipe_config);

-   /* hw state is inconsistent with the pipe A quirk */
-   if (crtc-pipe == PIPE_A  dev_priv-quirks  
QUIRK_PIPEA_FORCE)
+   /* hw state is inconsistent with the pipe quirk */
+   if ((crtc-pipe == PIPE_A  dev_priv-quirks  
QUIRK_PIPEA_FORCE) ||
+   (crtc-pipe == PIPE_B  dev_priv-quirks  
QUIRK_PIPEB_FORCE))
active = crtc-active;

for_each_intel_encoder(dev, encoder) {
@@ -12429,6 +12433,14 @@ 

Re: [Intel-gfx] [PATCH 16/16] drm/i915: Preserve VGACNTR bits from the BIOS

2014-08-15 Thread Thomas Richter

On 15.08.2014 00:22, ville.syrj...@linux.intel.com wrote:

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

My Fujistsu-Siemens Lifebook S6010 doesn't like to resume from
S3 unless VGACNTR has been restore to the original value. The BIOS
value in this case was 0x0124008E. Setting the VGA disable bit
doesn't interfere with the S3 resume fortunately.

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


Works on the S6010 as advertized, though cannot test on the R31 since it
does not resume from S3 - it does not even reach the real-mode entry 
hook of the kernel.


Tested-by: Thomas Richter rich...@rus.uni-stuttgart.de


---
  drivers/gpu/drm/i915/i915_drv.h  | 2 ++
  drivers/gpu/drm/i915/intel_display.c | 8 +++-
  2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1ed71e..e0f64e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1638,6 +1638,8 @@ struct drm_i915_private {
 */
struct workqueue_struct *dp_wq;

+   uint32_t bios_vgacntr;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
 * here! */
struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 92baf6f..f154993 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12591,7 +12591,11 @@ static void i915_disable_vga(struct drm_device *dev)
vga_put(dev-pdev, VGA_RSRC_LEGACY_IO);
udelay(300);

-   I915_WRITE(vga_reg, VGA_DISP_DISABLE);
+   /*
+* Fujitsu-Siemens Lifebook S6010 (830) has problems resuming
+* from S3 without preserving (some of?) the other bits.
+*/
+   I915_WRITE(vga_reg, dev_priv-bios_vgacntr | VGA_DISP_DISABLE);
POSTING_READ(vga_reg);
  }

@@ -12680,6 +12684,8 @@ void intel_modeset_init(struct drm_device *dev)

intel_shared_dpll_init(dev);

+   /* save the BIOS value before clobbering it */
+   dev_priv-bios_vgacntr = I915_READ(i915_vgacntrl_reg(dev));
/* Just disable it once at startup */
i915_disable_vga(dev);
intel_setup_outputs(dev);



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


Re: [Intel-gfx] [REGRESSION BISECTED] backlight control stops workin with 3.14 and later

2014-08-15 Thread Jani Nikula
On Thu, 14 Aug 2014, Bertrik Sikken bert...@sikken.nl wrote:
 Hi Jani,

 On 13-8-2014 3:43, Jani Nikula wrote:
 On Wed, 23 Jul 2014, Hans de Goede hdego...@redhat.com wrote:
 On 07/22/2014 08:52 AM, Daniel Vetter wrote:
 On Tue, Jul 22, 2014 at 8:42 AM, Hans de Goede hdego...@redhat.com wrote:

 snip

 Also please grab latest intel-gpu-tools and record a register dump
 with intel_reg_dump, again for both broken and working kernels.

 Bertrik, can you do this please (without the blacklisting or special
 kernel commandline options).
 
 Please attach dmesg with drm.debug=0xe module parameter set for some
 recent kernel.

 Attached is dmesg output from booting kernel 3.14-2 (debian unstable)
 with drm.debug=0xe and the samsung_laptop module enabled, from my
 Samsung N150plus netbook.

Have you tried 3.15?

BR,
Jani.



 If you need anything else, please let me know.
 Thanks for looking into this,

 Bertrik Sikken
 [0.00] CPU0 microcode updated early to revision 0x107, date = 
 2009-08-25
 [0.00] Initializing cgroup subsys cpuset
 [0.00] Initializing cgroup subsys cpu
 [0.00] Initializing cgroup subsys cpuacct
 [0.00] Linux version 3.14-2-amd64 (debian-ker...@lists.debian.org) 
 (gcc version 4.8.3 (Debian 4.8.3-7) ) #1 SMP Debian 3.14.15-2 (2014-08-09)
 [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-3.14-2-amd64 
 root=UUID=d72b9d1c-1e83-43c5-b5b9-e7ad03e8bd9d ro quiet init=/bin/systemd 
 drm.debug=0xe
 [0.00] e820: BIOS-provided physical RAM map:
 [0.00] BIOS-e820: [mem 0x-0x0009dbff] usable
 [0.00] BIOS-e820: [mem 0x0009dc00-0x0009] reserved
 [0.00] BIOS-e820: [mem 0x000ce000-0x000c] reserved
 [0.00] BIOS-e820: [mem 0x000dc000-0x000d] reserved
 [0.00] BIOS-e820: [mem 0x000e4000-0x000f] reserved
 [0.00] BIOS-e820: [mem 0x0010-0x7f5a] usable
 [0.00] BIOS-e820: [mem 0x7f5b-0x7f5b] ACPI 
 data
 [0.00] BIOS-e820: [mem 0x7f5c-0x7f5c2fff] ACPI NVS
 [0.00] BIOS-e820: [mem 0x7f5c3000-0x7fff] reserved
 [0.00] BIOS-e820: [mem 0xe000-0xefff] reserved
 [0.00] BIOS-e820: [mem 0xfec0-0xfec0] reserved
 [0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
 [0.00] BIOS-e820: [mem 0xff00-0x] reserved
 [0.00] NX (Execute Disable) protection: active
 [0.00] SMBIOS 2.5 present.
 [0.00] DMI: SAMSUNG ELECTRONICS CO., LTD. N150P/N210P/N220P  
 /N150P/N210P/N220P  , BIOS 02KY.M021.20101217.RHU 12/17/2010
 [0.00] e820: update [mem 0x-0x0fff] usable == reserved
 [0.00] e820: remove [mem 0x000a-0x000f] usable
 [0.00] No AGP bridge found
 [0.00] e820: last_pfn = 0x7f5b0 max_arch_pfn = 0x4
 [0.00] MTRR default type: uncachable
 [0.00] MTRR fixed ranges enabled:
 [0.00]   0-9 write-back
 [0.00]   A-B uncachable
 [0.00]   C-C write-protect
 [0.00]   D-D uncachable
 [0.00]   E-F write-protect
 [0.00] MTRR variable ranges enabled:
 [0.00]   0 base 0 mask 08000 write-back
 [0.00]   1 base 07F60 mask 0FFE0 uncachable
 [0.00]   2 base 07F80 mask 0FF80 uncachable
 [0.00]   3 disabled
 [0.00]   4 disabled
 [0.00]   5 disabled
 [0.00]   6 disabled
 [0.00]   7 disabled
 [0.00] x86 PAT enabled: cpu 0, old 0x7040600070406, new 
 0x7010600070106
 [0.00] found SMP MP-table at [mem 0x000f7d40-0x000f7d4f] mapped at 
 [880f7d40]
 [0.00] Base memory trampoline at [88097000] 97000 size 24576
 [0.00] init_memory_mapping: [mem 0x-0x000f]
 [0.00]  [mem 0x-0x000f] page 4k
 [0.00] BRK [0x01aaf000, 0x01aa] PGTABLE
 [0.00] BRK [0x01ab, 0x01ab0fff] PGTABLE
 [0.00] BRK [0x01ab1000, 0x01ab1fff] PGTABLE
 [0.00] init_memory_mapping: [mem 0x7f20-0x7f3f]
 [0.00]  [mem 0x7f20-0x7f3f] page 2M
 [0.00] BRK [0x01ab2000, 0x01ab2fff] PGTABLE
 [0.00] init_memory_mapping: [mem 0x7c00-0x7f1f]
 [0.00]  [mem 0x7c00-0x7f1f] page 2M
 [0.00] init_memory_mapping: [mem 0x0010-0x7bff]
 [0.00]  [mem 0x0010-0x001f] page 4k
 [0.00]  [mem 0x0020-0x7bff] page 2M
 [0.00] init_memory_mapping: [mem 0x7f40-0x7f5a]
 [0.00]  [mem 0x7f40-0x7f5a] page 4k
 [0.00] BRK [0x01ab3000, 0x01ab3fff] PGTABLE
 [0.00] RAMDISK: [mem 0x36548000-0x3729bfff]
 [0.00] ACPI: RSDP 000f7d10 24 (v02 

Re: [Intel-gfx] [PATCH] drm: fix plane rotation when restoring fbdev configuration

2014-08-15 Thread Thomas Wood
On 15 August 2014 11:22, Daniel Vetter dan...@ffwll.ch wrote:
 On Fri, Aug 15, 2014 at 10:48:05AM +0100, Thomas Wood wrote:
 On 15 August 2014 10:42, Thomas Wood thomas.w...@intel.com wrote:
  On 14 August 2014 17:02, Daniel Vetter dan...@ffwll.ch wrote:
  On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote:
  Make sure plane rotation is reset correctly when restoring the fbdev
  configuration by using drm_mode_plane_set_obj_prop. This calls the
  driver's set_property callback and ensures the rotation is actually
  changed.
 
  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236
  Signed-off-by: Thomas Wood thomas.w...@intel.com
 
  Commit message is missing the citation of the offending commit that
  introduced this. With that addressed this is
 
  Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 
  And please cc all the people involved in the offending commit next time
  around, too.
 
  The reset feature was introduced in commit 9783de2 (drm: Resetting
  rotation property), although it also looks like the issue was
  originally addressed in a previous version of the patch:
 
  http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html
 
  Although this version calls the driver's set_property directly rather
  than exporting drm_mode_plane_set_obj_prop.
 
  The final version of the patch contains a different change:

 http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html

 So there were actually two different v2 versions of the patch.

 Somehow I've thought this is for -fixes, but the patch is only in dinq. So
 merged, thanks.
 -Daniel

Is there any preference to calling set_property directly (as
originally fixed), as opposed to exporting and using
drm_mode_plane_set_obj_prop?

I have also pushed a patch for igt/kms_rotate_crc to add a testcase
for this issue:

http://lists.freedesktop.org/archives/intel-gfx/2014-August/050775.html





 
  -Daniel
  ---
   drivers/gpu/drm/drm_crtc.c  | 25 -
   drivers/gpu/drm/drm_fb_helper.c |  6 +++---
   include/drm/drm_crtc.h  |  3 +++
   3 files changed, 26 insertions(+), 8 deletions(-)
 
  diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
  index f09b752..95f330a 100644
  --- a/drivers/gpu/drm/drm_crtc.c
  +++ b/drivers/gpu/drm/drm_crtc.c
  @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct 
  drm_mode_object *obj,
return ret;
   }
 
  -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
  -   struct drm_property *property,
  -   uint64_t value)
  +/**
  + * drm_mode_plane_set_obj_prop - set the value of a property
  + * @plane: drm plane object to set property value for
  + * @property: property to set
  + * @val: value the property should be set to
  + *
  + * This functions sets a given property on a given plane object. This 
  function
  + * calls the driver's -set_property callback and changes the software 
  state of
  + * the property if the callback succeeds.
  + *
  + * Returns:
  + * Zero on success, error code on failure.
  + */
  +int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
  + struct drm_property *property,
  + uint64_t value)
   {
int ret = -EINVAL;
  - struct drm_plane *plane = obj_to_plane(obj);
  + struct drm_mode_object *obj = plane-base;
 
if (plane-funcs-set_property)
ret = plane-funcs-set_property(plane, property, value);
  @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct 
  drm_mode_object *obj,
 
return ret;
   }
  +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
 
   /**
* drm_mode_getproperty_ioctl - get the current value of a object's 
  property
  @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct 
  drm_device *dev, void *data,
ret = drm_mode_crtc_set_obj_prop(arg_obj, property, 
  arg-value);
break;
case DRM_MODE_OBJECT_PLANE:
  - ret = drm_mode_plane_set_obj_prop(arg_obj, property, 
  arg-value);
  + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj),
  +   property, arg-value);
break;
}
 
  diff --git a/drivers/gpu/drm/drm_fb_helper.c 
  b/drivers/gpu/drm/drm_fb_helper.c
  index 63d7b8e..0c0c39b 100644
  --- a/drivers/gpu/drm/drm_fb_helper.c
  +++ b/drivers/gpu/drm/drm_fb_helper.c
  @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper 
  *fb_helper)
drm_plane_force_disable(plane);
 
if (dev-mode_config.rotation_property) {
  - drm_object_property_set_value(plane-base,
  - dev-mode_config.rotation_property,
  - BIT(DRM_ROTATE_0));
  + drm_mode_plane_set_obj_prop(plane,
  +

Re: [Intel-gfx] [PATCH 11/43] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-15 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 10:22:01AM +, Daniel, Thomas wrote:
 
 
  -Original Message-
  From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
  Vetter
  Sent: Monday, August 11, 2014 3:30 PM
  To: Daniel, Thomas
  Cc: intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH 11/43] drm/i915/bdw: Render moot context
  reset and switch with Execlists
  
  On Thu, Jul 24, 2014 at 05:04:19PM +0100, Thomas Daniel wrote:
   From: Oscar Mateo oscar.ma...@intel.com
  
   These two functions make no sense in an Logical Ring Context 
   Execlists world.
  
   v2: We got rid of lrc_enabled and centralized everything in the
   sanitized i915.enbale_execlists instead.
  
   Signed-off-by: Oscar Mateo oscar.ma...@intel.com
   ---
drivers/gpu/drm/i915/i915_gem_context.c |9 +
1 file changed, 9 insertions(+)
  
   diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
   b/drivers/gpu/drm/i915/i915_gem_context.c
   index fbe7278..288f5de 100644
   --- a/drivers/gpu/drm/i915/i915_gem_context.c
   +++ b/drivers/gpu/drm/i915/i915_gem_context.c
   @@ -380,6 +380,9 @@ void i915_gem_context_reset(struct drm_device
  *dev)
 struct drm_i915_private *dev_priv = dev-dev_private;
 int i;
  
   + if (i915.enable_execlists)
   + return;
  
  This will conflict badly with Alistair's patch at a functional level. I'm 
  pretty sure
  that we want _some_ form of reset for the context state, since the hw didn't
  just magically load the previously running context. So NACK on this hunk.
 OK I'll wait to see the final version of Alistair's patch and decide what to 
 do
 about this hunk.
 
  
   +
 /* Prevent the hardware from restoring the last context (which
  hung) on
  * the next switch */
 for (i = 0; i  I915_NUM_RINGS; i++) { @@ -514,6 +517,9 @@ int
   i915_gem_context_enable(struct drm_i915_private *dev_priv)
 ppgtt-enable(ppgtt);
 }
  
   + if (i915.enable_execlists)
   + return 0;
  
  Again this conflicts with Alistair's patch. Furthermore it looks redudant 
  since
  you no-op out i915_switch_context separately.
 I don't think this is a conflict.  Doesn't Alistair's change here just involve
 writing PDPs for full PPGTT?  We don't want to do that in lrc mode.

Oh, just context conflict since Alistair's patch removes the next line ;-)
Also I shuffled some of the ppgtt code right above this around.

   +
 /* FIXME: We should make this work, even in reset */
 if (i915_reset_in_progress(dev_priv-gpu_error))
 return 0;
   @@ -769,6 +775,9 @@ int i915_switch_context(struct intel_engine_cs
   *ring,  {
 struct drm_i915_private *dev_priv = ring-dev-dev_private;
  
   + if (i915.enable_execlists)
   + return 0;
  
  I've hoped we don't need this with the legacy ringbuffer cmdsubmission fuly
  split out. If there are other paths (resume, gpu reset) where this comes 
  into
  play then I guess we need to look at where the best place is to make this 
  call.
  So until this comes with a bit a better justification I'll punt on this for 
  now.
  -Daniel
 Yes, the command submission lrc path doesn't call this but other codepaths
 do.  If we keep the check in context_enable() the only remaining call I see is
 in i915_gpu_idle().  I don't mind if the check is done there but perhaps a
 WARN_ON should then be added into switch_context() because we don't
 want to be putting illegal MI_SET_CONTEXT commands into the ring.

Yeah, that sounds like a plan. There's still a bit of confusion around the
ctx reset in Alistairs patch but unrelated to the code bits we've
discussed here. So when you resend this revised patch can you please merge
Alistair's patch into your baseline to avoid conflicts.

Thanks, 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 v2] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-08-15 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 3:33 PM, Mcaulay, Alistair
alistair.mcau...@intel.com wrote:
 below is the basic code path of a reset which has been changed by my patch:

 i915_reset()
 {
 
 i915_gem_reset() - This used to call i915_gem_context_reset(), which 
 has now been removed.
 .
 i915_gem_init_hw()
 .
 i915_gem_context_enable() - This used to return during 
 reset. Now it doesn't
 .
 for each ring, i915_switch_context(default)
 do_switch();
 .

 .
 }

  I am with Daniel on this one. I don't understand how can we throw 
 everything in here away.
 Did you maybe mean Ben?
 Daniel, I thought you were happy with the implementation, and V2 fixed your 
 last concern, could you please comment.

I'm happy with the underlying fix, but I didn't check all the details
and instead signed up Mika for that. Helps me scale and also makes
sure that more people understand tricky parts.

  We need to force hw to switch to a working context, after reset, so that 
 our internal state tracking matches.
 I believe this patch does that using i915_switch_context, rather than the 
 hacky i915_gem_context_reset()

I'm half in a plane already, so I'll leave it to you and Mika to
figure this out. Maybe the only thing missing is a bit more
explanation in the commit message why exactly we can remove all that
code. On a quick glance both Mika's concern and your reply make sense.
-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/bdw: Disable execlists by default

2014-08-15 Thread Damien Lespiau
We still have a few missing bits and pieces to have execlists enabled by
default eg. the error capture or the render state initialization and so
it wouldn't be wise to enable it by default on BDW just yet.

Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Thomas Daniel thomas.dan...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index 0886aca..139f490 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -35,7 +35,7 @@ struct i915_params i915 __read_mostly = {
.vbt_sdvo_panel_type = -1,
.enable_rc6 = -1,
.enable_fbc = -1,
-   .enable_execlists = -1,
+   .enable_execlists = 0,
.enable_hangcheck = true,
.enable_ppgtt = -1,
.enable_psr = 0,
@@ -122,7 +122,7 @@ MODULE_PARM_DESC(enable_ppgtt,
 module_param_named(enable_execlists, i915.enable_execlists, int, 0400);
 MODULE_PARM_DESC(enable_execlists,
Override execlists usage. 
-   (-1=auto [default], 0=disabled, 1=enabled));
+   (-1=auto, 0=disabled [default], 1=enabled));
 
 module_param_named(enable_psr, i915.enable_psr, int, 0600);
 MODULE_PARM_DESC(enable_psr, Enable PSR (default: false));
-- 
1.8.3.1

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


[Intel-gfx] [QA 08/15 ww33] Testing report for `drm-intel-testing` (was: Updated -next)

2014-08-15 Thread Sun, Yi
Summary
We covered the platform: Broadwell, Baytrail-M, Haswell (mobile, desktop and 
ULT), Ivybridge, SandyBridge, IronLake.
In this circle, 3 new bugs are filed.
   Bug 82611https://bugs.freedesktop.org/show_bug.cgi?id=82611 - [BDW 
Regression ]HDMI hot plug case ERROR.
Bug 82551https://bugs.freedesktop.org/show_bug.cgi?id=82551 - [HSW ] HDMI hot 
plug sporadically cause ERROR
   Bug 82593https://bugs.freedesktop.org/show_bug.cgi?id=82593 - [BYT]udevadm 
unable to monitor HDMI plug/unplug event sporadically

Finding
There's one blocking issue that system can't resume from S3 or S4. This issue 
influence all platforms. But we can't reproduce on latest -nightly, -fixes and 
-next-queued branch, so didn't file a bug report.

Top bugs which decrease the pass rate:
   Bug 81903https://bugs.freedesktop.org/show_bug.cgi?id=81903 - [BDW 
Bisected] DP Hot plug cause DP monitor can't be lighted up   (This bug blocks 
some BDW DP hot plug cases )
   Bug 81895https://bugs.freedesktop.org/show_bug.cgi?id=81895 - [BDW 
Bisected SST/MST] 4k resolution cannot be found when connected by MST (but is 
shown for SST) (This bug blocks BDW DP 4k display)
  Bug 81940https://bugs.freedesktop.org/show_bug.cgi?id=81940 - [BDW 
Bisected]eDP can't be lighted up after Xrandr rotate (This bug blocks all BDW 
rotation cases)

Test Environment
commit 0c6aad835dd1b817e87e312da0350e07952ff25e
Merge: b63401a 80b36f4
Author: Daniel Vetter daniel.vet...@ffwll.chmailto:daniel.vet...@ffwll.ch
Date:   Fri Aug 8 17:47:56 2014 +0200

Merge remote-tracking branch 'origin/topic/vblank-rework' into 
drm-intel-nightly


Kernel Bugs Status:

[cid:image003.png@01CFB8E9.B10A3280]


Thanks
  --Sun, Yi



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


Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended

2014-08-15 Thread Paulo Zanoni
2014-08-15 5:39 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com:
 On Thu, Aug 14, 2014 at 12:06:02PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com

 If we're runtime suspended and try to use the plane interfaces, we
 will get a lot of WARNs saying we did the wrong thing.

 We need to get runtime PM references to pin the objects, and to
 change the fences. The pin functions are the ideal places for
 this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
 have to add get/put calls inside it. There is no problem if we runtime
 suspend right after these functions are finished, because the
 registers written are forwarded to system memory.

 Note: for a complete fix of the cursor-dpms test case, we also need
 the patch named drm/i915: Don't try to enable cursor from setplane
 when crtc is disabled.

 v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
 v3: - Make get/put also surround the fence and unpin calls (Daniel and
   Ville).
 - Merge all the plane changes into a single patch since they're
   the same fix.
 - Add the comment requested by Daniel.
 v4: - Remove spurious whitespace (Ville).
 v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
   equivalent fix in another patch (Ville).
 v6: - Remove unpin chunk: it will be on a separate patch (Ville,
   Chris, Daniel).

 Testcase: igt/pm_rpm/cursor
 Testcase: igt/pm_rpm/cursor-dpms
 Testcase: igt/pm_rpm/legacy-planes
 Testcase: igt/pm_rpm/legacy-planes-dpms
 Testcase: igt/pm_rpm/universal-planes
 Testcase: igt/pm_rpm/universal-planes-dpms
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
 Cc: sta...@vger.kernel.org
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 27 +++
  1 file changed, 27 insertions(+)


 I talked with Daniel and we agreed to leave any possible fixes related with 
 the
 unpin functions to separate patches, possibly requiring separate IGT test
 cases.

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 3813526..17bc661 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
   if (need_vtd_wa(dev)  alignment  256 * 1024)
   alignment = 256 * 1024;

 + /*
 +  * Global gtt pte registers are special registers which actually 
 forward
 +  * writes to a chunk of system memory. Which means that there is no 
 risk
 +  * that the register values disappear as soon as we call
 +  * intel_runtime_pm_put(), so it is correct to wrap only the
 +  * pin/unpin/fence and not more.
 +  */
 + intel_runtime_pm_get(dev_priv);
 +
   dev_priv-mm.interruptible = false;
   ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
   if (ret)
 @@ -2166,12 +2175,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
   i915_gem_object_pin_fence(obj);

   dev_priv-mm.interruptible = true;
 + intel_runtime_pm_put(dev_priv);
   return 0;

  err_unpin:
   i915_gem_object_unpin_from_display_plane(obj);
  err_interruptible:
   dev_priv-mm.interruptible = true;
 + intel_runtime_pm_put(dev_priv);
   return ret;
  }

 @@ -8201,6 +8212,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
 *crtc,
uint32_t width, uint32_t height)
  {
   struct drm_device *dev = crtc-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   enum pipe pipe = intel_crtc-pipe;
   unsigned old_width, stride;
 @@ -8231,6 +8243,16 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
 *crtc,

   /* we only need to pin inside GTT if cursor is non-phy */
   mutex_lock(dev-struct_mutex);
 +
 + /*
 +  * Global gtt pte registers are special registers which actually 
 forward
 +  * writes to a chunk of system memory. Which means that there is no 
 risk
 +  * that the register values disappear as soon as we call
 +  * intel_runtime_pm_put(), so it is correct to wrap only the
 +  * pin/unpin/fence and not more.
 +  */
 + intel_runtime_pm_get(dev_priv);
 +

 Only the !cursor_needs_physical case need runtime pm get/put. I thought
 the calls were there originally, but I guess they got moved out. I
 suppose it's not a huge deal either way, but the current approach does
 give the reader the wrong impression that the unpin and frontbuffer
 tracking would also need a rpm reference.

It's not a huge deal either way, yet you don't give a reviewed-by tag :)

I thought about just putting the get/put inside cursor_needs_physical,
but then I'd need a new bool variable to track whether we need to put
or not at the failure code paths. And we have so much code churn that
maybe additional changes could leave the 

Re: [Intel-gfx] [PATCH] drm/i915: fix i915_frequency_info on BDW

2014-08-15 Thread Rodrigo Vivi
On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni przan...@gmail.com wrote:

 From: Paulo Zanoni paulo.r.zan...@intel.com

 The GEN6_PM* registers don't exist on BDW anymore, so when we read
 this file we trigger unclaimed register errors. The equivalent BDW
 register for PMs is GEN8_GT_I*R(2), so use it.

 Testcase: igt/pm_rpm/debugfs-read
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 9e737b7..17bd20ff 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
 void *unused)
 u32 rpstat, cagf, reqf;
 u32 rpupei, rpcurup, rpprevup;
 u32 rpdownei, rpcurdown, rpprevdown;
 +   u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
 int max_freq;

 /* RPSTAT1 is in the GT power well */
 @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file *m,
 void *unused)
 gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 mutex_unlock(dev-struct_mutex);

 +   if (IS_GEN6(dev) || IS_GEN7(dev)) {
 +   pm_ier = I915_READ(GEN6_PMIER);
 +   pm_imr = I915_READ(GEN6_PMIMR);
 +   pm_isr = I915_READ(GEN6_PMISR);
 +   pm_iir = I915_READ(GEN6_PMIIR);
 +   pm_mask = I915_READ(GEN6_PMINTRMSK);
 +   } else {
 +   pm_ier = I915_READ(GEN8_GT_IER(2));
 +   pm_imr = I915_READ(GEN8_GT_IMR(2));
 +   pm_isr = I915_READ(GEN8_GT_ISR(2));
 +   pm_iir = I915_READ(GEN8_GT_IIR(2));


Why do we care only about GT(2) interrupt reg?
What about other 0, 1 and 3 regs?

Could this explain GT3 failures?


 +   pm_mask = I915_READ(GEN6_PMINTRMSK);
 +   }
 seq_printf(m, PM IER=0x%08x IMR=0x%08x ISR=0x%08x
 IIR=0x%08x, MASK=0x%08x\n,
 -  I915_READ(GEN6_PMIER),
 -  I915_READ(GEN6_PMIMR),
 -  I915_READ(GEN6_PMISR),
 -  I915_READ(GEN6_PMIIR),
 -  I915_READ(GEN6_PMINTRMSK));
 +  pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
 seq_printf(m, GT_PERF_STATUS: 0x%08x\n, gt_perf_status);
 seq_printf(m, Render p-state ratio: %d\n,
(gt_perf_status  0xff00)  8);
 --
 2.0.1

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




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


Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended

2014-08-15 Thread Ville Syrjälä
On Fri, Aug 15, 2014 at 01:47:18PM -0300, Paulo Zanoni wrote:
 2014-08-15 5:39 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com:
  On Thu, Aug 14, 2014 at 12:06:02PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  If we're runtime suspended and try to use the plane interfaces, we
  will get a lot of WARNs saying we did the wrong thing.
 
  We need to get runtime PM references to pin the objects, and to
  change the fences. The pin functions are the ideal places for
  this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
  have to add get/put calls inside it. There is no problem if we runtime
  suspend right after these functions are finished, because the
  registers written are forwarded to system memory.
 
  Note: for a complete fix of the cursor-dpms test case, we also need
  the patch named drm/i915: Don't try to enable cursor from setplane
  when crtc is disabled.
 
  v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
  v3: - Make get/put also surround the fence and unpin calls (Daniel and
Ville).
  - Merge all the plane changes into a single patch since they're
the same fix.
  - Add the comment requested by Daniel.
  v4: - Remove spurious whitespace (Ville).
  v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
equivalent fix in another patch (Ville).
  v6: - Remove unpin chunk: it will be on a separate patch (Ville,
Chris, Daniel).
 
  Testcase: igt/pm_rpm/cursor
  Testcase: igt/pm_rpm/cursor-dpms
  Testcase: igt/pm_rpm/legacy-planes
  Testcase: igt/pm_rpm/legacy-planes-dpms
  Testcase: igt/pm_rpm/universal-planes
  Testcase: igt/pm_rpm/universal-planes-dpms
  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
  Cc: sta...@vger.kernel.org
  Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 27 +++
   1 file changed, 27 insertions(+)
 
 
  I talked with Daniel and we agreed to leave any possible fixes related 
  with the
  unpin functions to separate patches, possibly requiring separate IGT test
  cases.
 
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 3813526..17bc661 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
if (need_vtd_wa(dev)  alignment  256 * 1024)
alignment = 256 * 1024;
 
  + /*
  +  * Global gtt pte registers are special registers which actually 
  forward
  +  * writes to a chunk of system memory. Which means that there is no 
  risk
  +  * that the register values disappear as soon as we call
  +  * intel_runtime_pm_put(), so it is correct to wrap only the
  +  * pin/unpin/fence and not more.
  +  */
  + intel_runtime_pm_get(dev_priv);
  +
dev_priv-mm.interruptible = false;
ret = i915_gem_object_pin_to_display_plane(obj, alignment, 
  pipelined);
if (ret)
  @@ -2166,12 +2175,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
i915_gem_object_pin_fence(obj);
 
dev_priv-mm.interruptible = true;
  + intel_runtime_pm_put(dev_priv);
return 0;
 
   err_unpin:
i915_gem_object_unpin_from_display_plane(obj);
   err_interruptible:
dev_priv-mm.interruptible = true;
  + intel_runtime_pm_put(dev_priv);
return ret;
   }
 
  @@ -8201,6 +8212,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
  *crtc,
 uint32_t width, uint32_t height)
   {
struct drm_device *dev = crtc-dev;
  + struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc-pipe;
unsigned old_width, stride;
  @@ -8231,6 +8243,16 @@ static int intel_crtc_cursor_set_obj(struct 
  drm_crtc *crtc,
 
/* we only need to pin inside GTT if cursor is non-phy */
mutex_lock(dev-struct_mutex);
  +
  + /*
  +  * Global gtt pte registers are special registers which actually 
  forward
  +  * writes to a chunk of system memory. Which means that there is no 
  risk
  +  * that the register values disappear as soon as we call
  +  * intel_runtime_pm_put(), so it is correct to wrap only the
  +  * pin/unpin/fence and not more.
  +  */
  + intel_runtime_pm_get(dev_priv);
  +
 
  Only the !cursor_needs_physical case need runtime pm get/put. I thought
  the calls were there originally, but I guess they got moved out. I
  suppose it's not a huge deal either way, but the current approach does
  give the reader the wrong impression that the unpin and frontbuffer
  tracking would also need a rpm reference.
 
 It's not a huge deal either way, yet you don't give a reviewed-by tag :)

I wanted to read your reply if there's a good reason for it ;)

 
 

Re: [Intel-gfx] [PATCH v2] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-08-15 Thread Mika Kuoppala
Mcaulay, Alistair alistair.mcau...@intel.com writes:

 Hi Mika / Daniel,

 below is the basic code path of a reset which has been changed by my patch:

 i915_reset()
 {
   
   i915_gem_reset() - This used to call i915_gem_context_reset(), which 
 has now been removed. 
   .
   i915_gem_init_hw()
   .
   i915_gem_context_enable() - This used to return during reset. 
 Now it doesn't
   .
   for each ring, i915_switch_context(default)
   do_switch();
   .

   .
 }

  I am with Daniel on this one. I don't understand how can we throw 
 everything in here away.
 Did you maybe mean Ben?
 Daniel, I thought you were happy with the implementation, and V2 fixed your 
 last concern, could you please comment.

  We need to force hw to switch to a working context, after reset, so that 
 our internal state tracking matches.
 I believe this patch does that using i915_switch_context, rather than the 
 hacky i915_gem_context_reset()

Our internal state tracking will be ok after i915_gem_context_enable()
has been called. All rings have been set to the default context.

But what happens with this sequence:

- render ring was running in default context 
- reset happens
- we call i915_gem_context_enable 
- do_switch will get called 
- it figure out that last context is the same as we are switching to
  (from == to) and it bails out
- we never wrote anything to ring, so we have pre reset context running.
  MI_SET_CONTEXT was never run.

Even if reset would not clear the CCID, I think it is safest to
force a MI_SET_CONTEXT here.

Further, if the default context was mangled before the reset,
we should reinitialize it to a known state by running
i915_gem_render_state_init() for it. But this can be
considered as a possible future work.

-Mika

 Alistair.

 -Original Message-
 From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com]
 Sent: Wednesday, August 06, 2014 5:25 PM
 To: Mcaulay, Alistair; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Rework GPU reset sequence to
 match driver load  thaw
 
 
 Hi,
 
 alistair.mcau...@intel.com writes:
 
  From: McAulay, Alistair alistair.mcau...@intel.com
 
  This patch is to address Daniels concerns over different code during reset:
 
  http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
 
  The reason for aiming as hard as possible to use the exact same code
  for driver load, gpu reset and runtime pm/system resume is that we've
  simply seen too many bugs due to slight variations and unintended
 omissions.
 
  Tested using igt drv_hangman.
 
  V2: Cleaner way of preventing check_wedge returning -EAGAIN
 
  Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.c |  6 +++
   drivers/gpu/drm/i915/i915_drv.h |  3 ++
   drivers/gpu/drm/i915/i915_gem.c |  6 +--
   drivers/gpu/drm/i915/i915_gem_context.c | 42 +
   drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
  +
   drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
   6 files changed, 23 insertions(+), 104 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_drv.c
  b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644
  --- a/drivers/gpu/drm/i915/i915_drv.c
  +++ b/drivers/gpu/drm/i915/i915_drv.c
  @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev)
 !dev_priv-ums.mm_suspended) {
 dev_priv-ums.mm_suspended = 0;
 
  +  /* Used to prevent gem_check_wedged returning -EAGAIN
 during gpu reset */
  +  dev_priv-gpu_error.reload_in_reset = true;
  +
 ret = i915_gem_init_hw(dev);
  +
  +  dev_priv-gpu_error.reload_in_reset = false;
  +
 mutex_unlock(dev-struct_mutex);
 if (ret) {
 DRM_ERROR(Failed hw init on reset %d\n, ret); diff
 --git
  a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
  index 991b663..116daff 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -1217,6 +1217,9 @@ struct i915_gpu_error {
 
 /* For missed irq/seqno simulation. */
 unsigned int test_irq_rings;
  +
  +  /* Used to prevent gem_check_wedged returning -EAGAIN during
 gpu reset   */
  +  bool reload_in_reset;
   };
 
   enum modeset_restore {
  diff --git a/drivers/gpu/drm/i915/i915_gem.c
  b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..14e1770 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error
 *error,
 if (i915_terminally_wedged(error))
 return -EIO;
 
  -  return -EAGAIN;
  +  /* Check if GPU Reset is in progress */
  +  if (!error-reload_in_reset)
  +  return -EAGAIN;
 }
 
 return 

Re: [Intel-gfx] [PATCH] drm/i915: fix i915_frequency_info on BDW

2014-08-15 Thread Paulo Zanoni
2014-08-15 13:50 GMT-03:00 Rodrigo Vivi rodrigo.v...@gmail.com:



 On Fri, Aug 1, 2014 at 2:14 PM, Paulo Zanoni przan...@gmail.com wrote:

 From: Paulo Zanoni paulo.r.zan...@intel.com

 The GEN6_PM* registers don't exist on BDW anymore, so when we read
 this file we trigger unclaimed register errors. The equivalent BDW
 register for PMs is GEN8_GT_I*R(2), so use it.

 Testcase: igt/pm_rpm/debugfs-read
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 9e737b7..17bd20ff 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1024,6 +1024,7 @@ static int i915_frequency_info(struct seq_file *m,
 void *unused)
 u32 rpstat, cagf, reqf;
 u32 rpupei, rpcurup, rpprevup;
 u32 rpdownei, rpcurdown, rpprevdown;
 +   u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
 int max_freq;

 /* RPSTAT1 is in the GT power well */
 @@ -1061,12 +1062,21 @@ static int i915_frequency_info(struct seq_file *m,
 void *unused)
 gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 mutex_unlock(dev-struct_mutex);

 +   if (IS_GEN6(dev) || IS_GEN7(dev)) {
 +   pm_ier = I915_READ(GEN6_PMIER);
 +   pm_imr = I915_READ(GEN6_PMIMR);
 +   pm_isr = I915_READ(GEN6_PMISR);
 +   pm_iir = I915_READ(GEN6_PMIIR);
 +   pm_mask = I915_READ(GEN6_PMINTRMSK);
 +   } else {
 +   pm_ier = I915_READ(GEN8_GT_IER(2));
 +   pm_imr = I915_READ(GEN8_GT_IMR(2));
 +   pm_isr = I915_READ(GEN8_GT_ISR(2));
 +   pm_iir = I915_READ(GEN8_GT_IIR(2));


 Why do we care only about GT(2) interrupt reg?
 What about other 0, 1 and 3 regs?

Because, as far as I could see, the GEN8_GT(2) register is the one
that seems to be equivalent to the old GEN6_PM register in terms of
the functionality debugged by this function: it is the one that
contains all the RPS stuff. Another thing that influenced me to take
this decision was that, for example, snb_update_pm_irq() touches
GEN6_PM, while bdw_update_pm_irq() touches only GEN8_GT(2). But I'm
not a great user of this code, so maybe we do want more interrupts.
OTOH, if we want all, there's always i915_interrupt_info.


 Could this explain GT3 failures?

Which GT3 failures? I don't understand why you ask this.



 +   pm_mask = I915_READ(GEN6_PMINTRMSK);
 +   }
 seq_printf(m, PM IER=0x%08x IMR=0x%08x ISR=0x%08x
 IIR=0x%08x, MASK=0x%08x\n,
 -  I915_READ(GEN6_PMIER),
 -  I915_READ(GEN6_PMIMR),
 -  I915_READ(GEN6_PMISR),
 -  I915_READ(GEN6_PMIIR),
 -  I915_READ(GEN6_PMINTRMSK));
 +  pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
 seq_printf(m, GT_PERF_STATUS: 0x%08x\n, gt_perf_status);
 seq_printf(m, Render p-state ratio: %d\n,
(gt_perf_status  0xff00)  8);
 --
 2.0.1

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




 --
 Rodrigo Vivi
 Blog: http://blog.vivi.eng.br




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


Re: [Intel-gfx] [PATCH] drm/i915: Idleness detection for DRRS

2014-08-15 Thread Vandana Kannan
On Aug-07-2014 7:27 PM, Daniel Vetter wrote:
 Hi Vandana,
 
 I think we need to start over from a freshclean slate with this part
 - too much changed and all the differen revisions of this patch don't
 provide that much information.
 
 Also as Chris says this introduces yet another idleness detection
 system, and we already have plenty of them.
 
 High-level comments:
 
 - We now have the frontbuffer tracking infrastructure, see the various
 new intel_frontbuffer_* and similar functions in intel_display.c. They
 have nice kerneldoc explaining what exactly they do. We need to hook
 into that instead of creating new idlesness detection.
 
Hi Daniel,

Could you point me to this documentation? I cant find it - might be
missing something..

Apart from this idleness detection patch, there are 2 other patches
which just set the appropriate registers for bdw and vlv (in
set_drrs()). These 2 patches have got R-b from Jani and need rebasing. I
was wondering if I could resend those patches - they won't have an
effect until DRRS trigger is in place..

- Vandana
 - There's already calls to intel_increase/decrease_pllclock. That
 should be renamed to gmch_increase/decrease_pllclock, and we the new
 dp drrs support here should be called at _exactly_ the same spots.
 
 - This doesn't use the pipe config we've added at all - DRRS shouldn't
 look at the panel, but only at the pipe config to figure out whether
 drrs is supported on a given pipe or not. There is also no need to
 restrict DRRS to single pipe mode at all.
 
 - Locking is important. In case of doubt follow the example
 established by the new PSR code. Especially important is to not
 require modeset locks.
 
 - There's no need to disable DRRS from the PSR (or fbc) code at all,
 the frontbuffer tracking code will take care of all this. And just
 using pageflips as business signal isn't good enough.
 
 This is just a rough guideline, I think it'd be good to first hash out
 the rough design in more detail (maybe as a patch with just
 pseudo-code) before starting with codeing.
 
 Thanks, Daniel
 
 
 
 On Thu, Aug 7, 2014 at 3:45 PM, Vandana Kannan vandana.kan...@intel.com 
 wrote:
 Adding support to detect display idleness by tracking page flip from
 user space. Switch to low refresh rate is triggered after 2 seconds of
 idleness. The delay is configurable. If there is a page flip or call to
 update the plane, then high refresh rate is applied.
 The feature is not used in dual-display mode.

 v2: Chris Wilson's review comments incorporated.
 Modify idleness detection implementation to make it similar to the
 implementation of intel_update_fbc/intel_disable_fbc

 v3: Internal review comments incorporated
 Add NULL pointer check in intel_disable_drrs.
 Add drrs calls in i9xx_crtc_enable/disable and valleyview_crtc_enable.

 v4: Jani's review comments incorporated.
 Change in sequence in intel_update_drrs. Comment modified to remove details
 of update param. Modified DRRS idleness interval to a module parameter.

 v5: Chris's review comments incorporated.
 Initialize connector in idleness detection init. Modifications made to
 use only intel_connector in i915_drrs and derive intel_dp when required.
 Added a function drrs_fini to cleanup DRRS work.

 v6: Internal review comments. Removed check for primary enabled, which is
 a redundant check, in the case of clone mode. Added a flag to track
 dual-display configuration. Remove print statement for cancel DRR work
 and print DRRS not supported only once.

 v7: As per internal review comments, removing calls to update/disable drrs
 from sprite update path. For sprite, all drrs related updates would be
 taken care of with calls to crtc page flip itself. This will have to be
 revisited later if flip infrastructure changes for sprite.

 v8: Incorporated Jani's review comments. Added space after the periods in the
 module param description. Changes around drrs-fini to remove seamless DRRS
 check.

 v9: Added checks for PSR before updating DRRS. Added check for module
 param drrs_interval before updating DRRS (this is required if the interval
 is modified by the user during system use). DRRS disabled by default. Changes
 based on Jani's review comments

 v10: Disable/enable DRRS when PSR is enable/disabled.

 v11: Moved DRRS not supported log to patch2.

 Patch rebased.

 Signed-off-by: Vandana Kannan vandana.kan...@intel.com
 Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
 Reviewed-by: Jani Nikula jani.nik...@intel.com
 Cc: Daniel Vetter dan...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_drv.h  |   7 ++
  drivers/gpu/drm/i915/i915_params.c   |   8 ++
  drivers/gpu/drm/i915/intel_display.c |  13 
  drivers/gpu/drm/i915/intel_dp.c  |  27 ++-
  drivers/gpu/drm/i915/intel_drv.h |   5 +-
  drivers/gpu/drm/i915/intel_pm.c  | 142 
 +++
  6 files changed, 200 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h 
 b/drivers/gpu/drm/i915/i915_drv.h
 index 

[Intel-gfx] [PATCH] drm/i915: Use dev_priv as first argument of for_each_pipe()

2014-08-15 Thread Damien Lespiau
Chris has decided that enough is enough. It's time to fixup dev Vs
dev_priv and the, oh so awful, INTEL_INFO(). This is a modest
contribution to the crusade.

Suggested-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 10 +++---
 drivers/gpu/drm/i915/i915_dma.c  |  4 +--
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/i915_irq.c  | 65 ++--
 drivers/gpu/drm/i915/intel_display.c | 16 +
 drivers/gpu/drm/i915/intel_dp.c  |  2 +-
 drivers/gpu/drm/i915/intel_panel.c   |  2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 17 +-
 8 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 6c82bda..637143c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -662,7 +662,7 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
   I915_READ(VLV_IIR_RW));
seq_printf(m, Display IMR:\t%08x\n,
   I915_READ(VLV_IMR));
-   for_each_pipe(pipe)
+   for_each_pipe(dev_priv, pipe)
seq_printf(m, Pipe %c stat:\t%08x\n,
   pipe_name(pipe),
   I915_READ(PIPESTAT(pipe)));
@@ -702,7 +702,7 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
   i, I915_READ(GEN8_GT_IER(i)));
}
 
-   for_each_pipe(pipe) {
+   for_each_pipe(dev_priv, pipe) {
if (!intel_display_power_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe))) {
seq_printf(m, Pipe %c power disabled\n,
@@ -749,7 +749,7 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
   I915_READ(VLV_IIR_RW));
seq_printf(m, Display IMR:\t%08x\n,
   I915_READ(VLV_IMR));
-   for_each_pipe(pipe)
+   for_each_pipe(dev_priv, pipe)
seq_printf(m, Pipe %c stat:\t%08x\n,
   pipe_name(pipe),
   I915_READ(PIPESTAT(pipe)));
@@ -785,7 +785,7 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
   I915_READ(IIR));
seq_printf(m, Interrupt mask:  %08x\n,
   I915_READ(IMR));
-   for_each_pipe(pipe)
+   for_each_pipe(dev_priv, pipe)
seq_printf(m, Pipe %c stat: %08x\n,
   pipe_name(pipe),
   I915_READ(PIPESTAT(pipe)));
@@ -4178,7 +4178,7 @@ void intel_display_crc_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
enum pipe pipe;
 
-   for_each_pipe(pipe) {
+   for_each_pipe(dev_priv, pipe) {
struct intel_pipe_crc *pipe_crc = dev_priv-pipe_crc[pipe];
 
pipe_crc-opened = false;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3f676f9..f19dbff 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1528,10 +1528,10 @@ static void intel_device_info_runtime_init(struct 
drm_device *dev)
info = (struct intel_device_info *)dev_priv-info;
 
if (IS_VALLEYVIEW(dev))
-   for_each_pipe(pipe)
+   for_each_pipe(dev_priv, pipe)
info-num_sprites[pipe] = 2;
else
-   for_each_pipe(pipe)
+   for_each_pipe(dev_priv, pipe)
info-num_sprites[pipe] = 1;
 
if (i915.disable_display) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4754328..e07419d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -163,7 +163,7 @@ enum hpd_pin {
 I915_GEM_DOMAIN_INSTRUCTION | \
 I915_GEM_DOMAIN_VERTEX)
 
-#define for_each_pipe(p) for ((p) = 0; (p)  INTEL_INFO(dev)-num_pipes; (p)++)
+#define for_each_pipe(dev, p) for ((p) = 0; (p)  (dev)-info.num_pipes; (p)++)
 #define for_each_sprite(p, s) for ((s) = 0; (s)  
INTEL_INFO(dev)-num_sprites[(p)]; (s)++)
 
 #define for_each_crtc(dev, crtc) \
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6b6fff1..06bb8cd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -238,7 +238,7 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
 
assert_spin_locked(dev_priv-irq_lock);
 
-   for_each_pipe(pipe) {
+   for_each_pipe(dev_priv, pipe) {
crtc = to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
 

[Intel-gfx] [PATCH v3] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-08-15 Thread alistair . mcaulay
From: McAulay, Alistair alistair.mcau...@intel.com

This patch is to address Daniels concerns over different code during reset:

http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html

The reason for aiming as hard as possible to use the exact same code for
driver load, gpu reset and runtime pm/system resume is that we've simply
seen too many bugs due to slight variations and unintended omissions.

Tested using igt drv_hangman.

V2: Cleaner way of preventing check_wedge returning -EAGAIN
V3: Clean the last_context during reset, to ensure do_switch() does the 
MI_SET_CONTEXT. As per review.
Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c |  6 +++
 drivers/gpu/drm/i915/i915_drv.h |  3 ++
 drivers/gpu/drm/i915/i915_gem.c |  4 +-
 drivers/gpu/drm/i915/i915_gem_context.c | 33 +++-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 67 +
 drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
 6 files changed, 28 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5e4fefd..3bfafe6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev)
!dev_priv-ums.mm_suspended) {
dev_priv-ums.mm_suspended = 0;
 
+   /* Used to prevent gem_check_wedged returning -EAGAIN during 
gpu reset */
+   dev_priv-gpu_error.reload_in_reset = true;
+
ret = i915_gem_init_hw(dev);
+
+   dev_priv-gpu_error.reload_in_reset = false;
+
mutex_unlock(dev-struct_mutex);
if (ret) {
DRM_ERROR(Failed hw init on reset %d\n, ret);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 991b663..116daff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1217,6 +1217,9 @@ struct i915_gpu_error {
 
/* For missed irq/seqno simulation. */
unsigned int test_irq_rings;
+
+   /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset  
 */
+   bool reload_in_reset;
 };
 
 enum modeset_restore {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ef047bc..e7396eb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
if (i915_terminally_wedged(error))
return -EIO;
 
-   return -EAGAIN;
+   /* Check if GPU Reset is in progress */
+   if (!error-reload_in_reset)
+   return -EAGAIN;
}
 
return 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index de72a28..9378ad8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -377,34 +377,17 @@ void i915_gem_context_reset(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
int i;
 
-   /* Prevent the hardware from restoring the last context (which hung) on
-* the next switch */
for (i = 0; i  I915_NUM_RINGS; i++) {
struct intel_engine_cs *ring = dev_priv-ring[i];
-   struct intel_context *dctx = ring-default_context;
struct intel_context *lctx = ring-last_context;
 
-   /* Do a fake switch to the default context */
-   if (lctx == dctx)
-   continue;
-
-   if (!lctx)
-   continue;
+   if (lctx) {
+   if (lctx-legacy_hw_ctx.rcs_state  i == RCS)
+   
i915_gem_object_ggtt_unpin(lctx-legacy_hw_ctx.rcs_state);
 
-   if (dctx-legacy_hw_ctx.rcs_state  i == RCS) {
-   
WARN_ON(i915_gem_obj_ggtt_pin(dctx-legacy_hw_ctx.rcs_state,
- 
get_context_alignment(dev), 0));
-   /* Fake a finish/inactive */
-   dctx-legacy_hw_ctx.rcs_state-base.write_domain = 0;
-   dctx-legacy_hw_ctx.rcs_state-active = 0;
+   i915_gem_context_unreference(lctx);
+   ring-last_context = NULL;
}
-
-   if (lctx-legacy_hw_ctx.rcs_state  i == RCS)
-   
i915_gem_object_ggtt_unpin(lctx-legacy_hw_ctx.rcs_state);
-
-   i915_gem_context_unreference(lctx);
-   i915_gem_context_reference(dctx);
-   ring-last_context = dctx;
}
 }
 
@@ -498,10 +481,6 @@ int i915_gem_context_enable(struct drm_i915_private 
*dev_priv)
ppgtt-enable(ppgtt);
}
 
-   /* FIXME: We should make this work, even in reset 

[Intel-gfx] [PULL] first drm-intel-next for 3.18

2014-08-15 Thread Daniel Vetter
Hi Dave,

So you made noises that you wanted to open drm-next right after -rc1, so
I've figured I'll test this ;-)

drm-intel-next-2014-08-08:
- Setting dp M2/N2 values plus state checker support (Vandana Kannan)
- chv power well support (Ville)
- DP training pattern 3 support for chv (Ville)
- cleanup of the hsw/bdw ddi pll code, prep work for skl (Damien)
- dsi video burst mode support (Shobhit)
- piles of other chv fixes all over (Ville et. al.)
- cleanup of the ddi translation tables setup code (Damien)
- 180 deg rotation support (Ville  Sonika Jindal)

Cheers, Daniel


The following changes since commit a91576d7916f6cce76d30303e60e1ac47cf4a76d:

  drm/ttm: Pass GFP flags in order to avoid deadlock. (2014-08-05 10:54:19 
+1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2014-08-08

for you to fetch changes up to 2c0827cffca8ac0c654b888c58a1989a5172f007:

  drm/i915: Update DRIVER_DATE to 20140808 (2014-08-08 20:44:59 +0200)


- Setting dp M2/N2 values plus state checker support (Vandana Kannan)
- chv power well support (Ville)
- DP training pattern 3 support for chv (Ville)
- cleanup of the hsw/bdw ddi pll code, prep work for skl (Damien)
- dsi video burst mode support (Shobhit)
- piles of other chv fixes all over (Ville et. al.)
- cleanup of the ddi translation tables setup code (Damien)
- 180 deg rotation support (Ville  Sonika Jindal)


Damien Lespiau (15):
  drm/i915: Specify when the PLL hw state fields are valid
  drm/i915: Add a space to the shared DPLL debug message
  drm/i915: Extract the HSW DDI selection code into its own function
  drm/i915: Extract the HSW/BDW shared dpll init code
  drm/i915: Restrict hsw_dp_set_ddi_pll_sel() to HSW/BDW
  drm/i915: Fix stale comment for intel_ddi_pll_select()
  drm/i915: Split the BDW/HSW specific shared pll selection
  drm/i915: Make intel_ddi_calculate_wrpll() HSW/BDW specific
  drm/i915: Split the CDCLK retrieval per-platform
  drm/i915: Gather the HDMI level shifter logic into one place
  drm/i915/bdw: Provide the BDW specific HDMI buffer translation table
  drm/i915/bdw: Remove the HDMI/DVI entry from the DP/eDP/FDI tables
  drm/i915: Remove now useless comments about the translation values
  drm/i915: Demote the DRRS messages to debug messages
  drm/i915: Introduce a for_each_intel_encoder() macro

Daniel Vetter (8):
  drm/i915: Update DRIVER_DATE to 20140725
  drm/i915: Don't require dev-struct_mutex in psr_match_conditions
  drm/i915: Tune done rc6 enabling output
  drm/i915: Tune down MCH_SSKPD values warning
  drm/i915: Make ddi_clock_gate() HSW/BDW specific
  drm/i915: Align intel_dsi*.c files a bit
  drm/i915: No busy-loop wait_for in the ring init code
  drm/i915: Update DRIVER_DATE to 20140808

Deepak S (1):
  drm/i915: Bring GPU Freq to min while suspending.

Gajanan Bhat (4):
  drm/i915: Update DDL only for current CRTC
  drm/i915: Generalize drain latency computation
  drm/i915: Round-up clock and limit drain latency
  drm/i915: Add sprite watermark programming for VLV and CHV

Imre Deak (2):
  drm/i915: factor out intel_edp_panel_vdd_sanitize
  drm/i915: fix VDD state tracking after system resume

Jesse Barnes (1):
  drm/i915: clean up PPGTT checking logic

Jiri Kosina (1):
  drm/i915: read HEAD register back in init_ring_common() to enforce 
ordering

Kenneth Graunke (2):
  drm/i915: Refactor Broadwell PIPE_CONTROL emission into a helper.
  drm/i915: Add the WaCsStallBeforeStateCacheInvalidate:bdw workaround.

Mika Kuoppala (1):
  drm/i915: Don't accumulate hangcheck score on forward progress

Paulo Zanoni (2):
  d rm/i915: freeze display before the interrupts and GT
  drm/i915: remove duplicate register defines

Pavel Machek (1):
  drm/i915: work around warning in i915_gem_gtt

Rafael Barbalho (2):
  drm/i915: Fix crash when failing to parse MIPI VBT
  drm/i915: Fix read back of plane stride register

Rodrigo Vivi (5):
  drm/i915: Fix error state collecting
  drm/i915: Collect gtier properly on HSW.
  drm/i915: Fix DEIER and GTIER collecting for BDW.
  Revert drm/i915: Enable semaphores on BDW
  drm/i915: Introduce FBC False Color for debug purposes.

Shobhit Kumar (3):
  drm/i915: wait for all DSI FIFOs to be empty
  drm/i915: Add correct hw/sw config check for DSI encoder
  drm/i915: Add support for Video Burst Mode for MIPI DSI

Sonika Jindal (2):
  drm: Add rotation_property to mode_config
  drm: Resetting rotation property

Vandana Kannan (2):
  drm/i915: Set M2_N2 registers during mode set
  drm/i915: State readout and cross-checking for dp_m2_n2

Ville Syrjälä (27):
  drm/i915: Fix threshold for choosing 32 vs. 64 precisions for VLV 

[Intel-gfx] [PULL] topic/core-stuff for 3.18

2014-08-15 Thread Daniel Vetter
Hi Dave,

So small drm stuff all over for 3.18. Biggest one is the cmdline parsing
from Chris with a few fixes from me to make it work for stupid kernel
configs.

Plus the atomic prep series.

Tested for more than a week in -nightly and Ville/Imre indeed discovered
some fun which is now fixed (and i915 vblank patches postponed since the
fixups need this branch plus drm-intel-next merged together).

Cheers, Daniel


The following changes since commit 168c02ec06f891990617cee2abbba70858c071e7:

  drm: Fix race when checking for fb in the generic kms obj lookup (2014-08-05 
11:06:52 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-08-15

for you to fetch changes up to 14f476fa24e81d0beea1aa14d763102958518d60:

  drm: Use the type of the array element when reallocating (2014-08-14 21:24:30 
+0200)


Chris Wilson (1):
  drm: Perform cmdline mode parsing during connector initialisation

Damien Lespiau (2):
  drm: Don't return 0 for a value used as a denominator
  drm: Use the type of the array element when reallocating

Daniel Vetter (10):
  drm: Don't grab an fb reference for the idr
  video/fbdev: Always built-in video= cmdline parsing
  drm: Add drm_plane/connector_index
  drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc]
  drm: Handle legacy per-crtc locking with full acquire ctx
  drm: Move -old_fb from crtc to plane
  drm: trylock modest locking for fbdev panics
  drm: Add a plane-reset hook
  drm/irq: Implement a generic vblank_wait function
  drm: Docbook fixes

Rob Clark (1):
  drm: idiot-proof vblank

Ville Syrjälä (1):
  drm: Warn when leaking flip events on close

 drivers/gpu/drm/Kconfig|   1 +
 drivers/gpu/drm/drm_crtc.c | 306 ++---
 drivers/gpu/drm/drm_dp_mst_topology.c  |   3 +-
 drivers/gpu/drm/drm_edid.c |   2 +-
 drivers/gpu/drm/drm_fb_helper.c|  76 +---
 drivers/gpu/drm/drm_fops.c |   2 +
 drivers/gpu/drm/drm_irq.c  |  68 
 drivers/gpu/drm/drm_modes.c|   1 +
 drivers/gpu/drm/drm_modeset_lock.c | 213 ++-
 drivers/gpu/drm/drm_probe_helper.c |  17 ++
 drivers/gpu/drm/mgag200/mgag200_mode.c |  21 +--
 drivers/video/fbdev/Kconfig|   4 +
 drivers/video/fbdev/core/Makefile  |   1 +
 drivers/video/fbdev/core/fb_cmdline.c  | 110 
 drivers/video/fbdev/core/fbmem.c   |  92 --
 drivers/video/fbdev/core/modedb.c  |   3 -
 include/drm/drmP.h |   2 +
 include/drm/drm_crtc.h |  22 ++-
 include/drm/drm_fb_helper.h|   1 -
 include/drm/drm_modeset_lock.h |  16 ++
 20 files changed, 630 insertions(+), 331 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

-- 
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 2/5] drm/i915: Do not access stolen memory directly by the CPU, even for error capture

2014-08-15 Thread Mika Kuoppala
Mika Kuoppala mika.kuopp...@linux.intel.com writes:

 Chris Wilson ch...@chris-wilson.co.uk writes:

 For stolen pages, since it is verboten to access them directly on many
 architectures, we have to read them through the GTT aperture. If they
 are not accessible through the aperture, then we have to abort.

 This was complicated by

 commit 8b6124a633d8095b0c8364f585edff9c59568a96
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Thu Jan 30 14:38:16 2014 +

 drm/i915: Don't access snooped pages through the GTT (even for error 
 capture)

 and the desire to use stolen memory for ringbuffers, contexts and
 batches in the future.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_gpu_error.c | 50 
 ++-
  1 file changed, 31 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 35e70d5..6d280c07 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -561,10 +561,11 @@ static struct drm_i915_error_object *
  i915_error_object_create_sized(struct drm_i915_private *dev_priv,
 struct drm_i915_gem_object *src,
 struct i915_address_space *vm,
 -   const int num_pages)
 +   int num_pages)
  {
  struct drm_i915_error_object *dst;
 -int i;
 +bool use_ggtt;
 +int i = 0;
  u32 reloc_offset;
  
  if (src == NULL || src-pages == NULL)
 @@ -574,8 +575,32 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
  if (dst == NULL)
  return NULL;
  
 -reloc_offset = dst-gtt_offset = i915_gem_obj_offset(src, vm);
 -for (i = 0; i  num_pages; i++) {
 +dst-gtt_offset = i915_gem_obj_offset(src, vm);
 +
 +reloc_offset = dst-gtt_offset;
 +use_ggtt = (src-cache_level == I915_CACHE_NONE 
  
 Take this cache level check out so that we end up doing the
 snoopable check for non stolen objects too?

This _is_ the snoopable check for non stolen objects.

Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com


 -Mika

 +i915_is_ggtt(vm) 
 +src-has_global_gtt_mapping 
 +reloc_offset + num_pages * PAGE_SIZE = 
 dev_priv-gtt.mappable_end);
 +
 +/* Cannot access stolen address directly, try to use the aperture */
 +if (src-stolen) {
 +use_ggtt = true;
 +
 +if (!src-has_global_gtt_mapping)
 +goto unwind;
 +
 +reloc_offset = i915_gem_obj_ggtt_offset(src);
 +if (reloc_offset + num_pages * PAGE_SIZE  
 dev_priv-gtt.mappable_end)
 +goto unwind;
 +}
 +
 +/* Cannot access snooped pages through the aperture */
 +if (use_ggtt  src-cache_level != I915_CACHE_NONE  
 !HAS_LLC(dev_priv-dev))
 +goto unwind;
 +
 +dst-page_count = num_pages;
 +while (num_pages--) {
  unsigned long flags;
  void *d;
  
 @@ -584,10 +609,7 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
  goto unwind;
  
  local_irq_save(flags);
 -if (src-cache_level == I915_CACHE_NONE 
 -reloc_offset  dev_priv-gtt.mappable_end 
 -src-has_global_gtt_mapping 
 -i915_is_ggtt(vm)) {
 +if (use_ggtt) {
  void __iomem *s;
  
  /* Simply ignore tiling or any overlapping fence.
 @@ -599,14 +621,6 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
   reloc_offset);
  memcpy_fromio(d, s, PAGE_SIZE);
  io_mapping_unmap_atomic(s);
 -} else if (src-stolen) {
 -unsigned long offset;
 -
 -offset = dev_priv-mm.stolen_base;
 -offset += src-stolen-start;
 -offset += i  PAGE_SHIFT;
 -
 -memcpy_fromio(d, (void __iomem *) offset, PAGE_SIZE);
  } else {
  struct page *page;
  void *s;
 @@ -623,11 +637,9 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
  }
  local_irq_restore(flags);
  
 -dst-pages[i] = d;
 -
 +dst-pages[i++] = d;
  reloc_offset += PAGE_SIZE;
  }
 -dst-page_count = num_pages;
  
  return dst;
  
 -- 
 1.9.1

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

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove num_pages parameter to i915_error_object_create()

2014-08-15 Thread Mika Kuoppala
Chris Wilson ch...@chris-wilson.co.uk writes:

 For cleanliness, i915_error_object_create() was written to handle the
 NULL pointer in a central location. The macro that wrapped it and passed
 it a num_pages to use, was not safe. As we now never limit the num_pages
 to use (we did so at one point to only capture the first page of the
 context), we can remove the redundant macro and be NULL safe again.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Cc: John Harrison john.c.harri...@intel.com

Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com



 ---
  drivers/gpu/drm/i915/i915_gpu_error.c | 25 ++---
  1 file changed, 10 insertions(+), 15 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 6d280c07..726e6b1 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -558,12 +558,12 @@ static void i915_error_state_free(struct kref 
 *error_ref)
  }
  
  static struct drm_i915_error_object *
 -i915_error_object_create_sized(struct drm_i915_private *dev_priv,
 -struct drm_i915_gem_object *src,
 -struct i915_address_space *vm,
 -int num_pages)
 +i915_error_object_create(struct drm_i915_private *dev_priv,
 +  struct drm_i915_gem_object *src,
 +  struct i915_address_space *vm)
  {
   struct drm_i915_error_object *dst;
 + int num_pages;
   bool use_ggtt;
   int i = 0;
   u32 reloc_offset;
 @@ -571,6 +571,8 @@ i915_error_object_create_sized(struct drm_i915_private 
 *dev_priv,
   if (src == NULL || src-pages == NULL)
   return NULL;
  
 + num_pages = src-base.size  PAGE_SHIFT;
 +
   dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *), GFP_ATOMIC);
   if (dst == NULL)
   return NULL;
 @@ -649,13 +651,8 @@ unwind:
   kfree(dst);
   return NULL;
  }
 -#define i915_error_object_create(dev_priv, src, vm) \
 - i915_error_object_create_sized((dev_priv), (src), (vm), \
 -(src)-base.sizePAGE_SHIFT)
 -
  #define i915_error_ggtt_object_create(dev_priv, src) \
 - i915_error_object_create_sized((dev_priv), (src), 
 (dev_priv)-gtt.base, \
 -(src)-base.sizePAGE_SHIFT)
 + i915_error_object_create((dev_priv), (src), (dev_priv)-gtt.base)
  
  static void capture_bo(struct drm_i915_error_buffer *err,
  struct i915_vma *vma)
 @@ -1004,8 +1001,7 @@ static void i915_gem_record_rings(struct drm_device 
 *dev,
request-batch_obj,
vm);
  
 - if (HAS_BROKEN_CS_TLB(dev_priv-dev) 
 - ring-scratch.obj)
 + if (HAS_BROKEN_CS_TLB(dev_priv-dev))
   error-ring[i].wa_batchbuffer =
   i915_error_ggtt_object_create(dev_priv,
ring-scratch.obj);
 @@ -1027,9 +1023,8 @@ static void i915_gem_record_rings(struct drm_device 
 *dev,
   error-ring[i].ringbuffer =
   i915_error_ggtt_object_create(dev_priv, 
 ring-buffer-obj);
  
 - if (ring-status_page.obj)
 - error-ring[i].hws_page =
 - i915_error_ggtt_object_create(dev_priv, 
 ring-status_page.obj);
 + error-ring[i].hws_page =
 + i915_error_ggtt_object_create(dev_priv, 
 ring-status_page.obj);
  
   i915_gem_record_active_context(ring, error, error-ring[i]);
  
 -- 
 1.9.1

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


Re: [Intel-gfx] [PATCH 4/5] drm/i915: Suppress a WARN on reading an object back for a GPU hang

2014-08-15 Thread Mika Kuoppala
Chris Wilson ch...@chris-wilson.co.uk writes:

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com



 ---
  drivers/gpu/drm/i915/i915_gpu_error.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
 b/drivers/gpu/drm/i915/i915_gpu_error.c
 index 726e6b1..1e05414 100644
 --- a/drivers/gpu/drm/i915/i915_gpu_error.c
 +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
 @@ -577,7 +577,10 @@ i915_error_object_create(struct drm_i915_private 
 *dev_priv,
   if (dst == NULL)
   return NULL;
  
 - dst-gtt_offset = i915_gem_obj_offset(src, vm);
 + if (i915_gem_obj_bound(src, vm))
 + dst-gtt_offset = i915_gem_obj_offset(src, vm);
 + else
 + dst-gtt_offset = -1;
  
   reloc_offset = dst-gtt_offset;
   use_ggtt = (src-cache_level == I915_CACHE_NONE 
 -- 
 1.9.1

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


Re: [Intel-gfx] [PATCH] drm/i915: Use dev_priv as first argument of for_each_pipe()

2014-08-15 Thread Chris Wilson
On Fri, Aug 15, 2014 at 06:34:06PM +0100, Damien Lespiau wrote:
 -#define for_each_pipe(p) for ((p) = 0; (p)  INTEL_INFO(dev)-num_pipes; 
 (p)++)
 +#define for_each_pipe(dev, p) for ((p) = 0; (p)  (dev)-info.num_pipes; 
 (p)++)
  #define for_each_sprite(p, s) for ((s) = 0; (s)  
 INTEL_INFO(dev)-num_sprites[(p)]; (s)++)

I think we still want to keep the INTEL_INFO() as that seems to be the
favourite for eliminating deadcode when compiling for a subset of
generation. It doesn't hurt to keep it around for a little longer.
-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


[Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended

2014-08-15 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

If we're runtime suspended and try to use the plane interfaces, we
will get a lot of WARNs saying we did the wrong thing.

We need to get runtime PM references to pin the objects, and to
change the fences. The pin functions are the ideal places for
this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
have to add get/put calls inside it. There is no problem if we runtime
suspend right after these functions are finished, because the
registers written are forwarded to system memory.

Note: for a complete fix of the cursor-dpms test case, we also need
the patch named drm/i915: Don't try to enable cursor from setplane
when crtc is disabled.

v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
v3: - Make get/put also surround the fence and unpin calls (Daniel and
  Ville).
- Merge all the plane changes into a single patch since they're
  the same fix.
- Add the comment requested by Daniel.
v4: - Remove spurious whitespace (Ville).
v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
  equivalent fix in another patch (Ville).
v6: - Remove unpin chunk: it will be on a separate patch (Ville,
  Chris, Daniel).
v7: - Same thing, new color.

Testcase: igt/pm_rpm/cursor
Testcase: igt/pm_rpm/cursor-dpms
Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/legacy-planes-dpms
Testcase: igt/pm_rpm/universal-planes
Testcase: igt/pm_rpm/universal-planes-dpms
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82603
Cc: sta...@vger.kernel.org
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3f8e037..15fe3eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2201,6 +2201,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
if (need_vtd_wa(dev)  alignment  256 * 1024)
alignment = 256 * 1024;
 
+   /*
+* Global gtt pte registers are special registers which actually forward
+* writes to a chunk of system memory. Which means that there is no risk
+* that the register values disappear as soon as we call
+* intel_runtime_pm_put(), so it is correct to wrap only the
+* pin/unpin/fence and not more.
+*/
+   intel_runtime_pm_get(dev_priv);
+
dev_priv-mm.interruptible = false;
ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
if (ret)
@@ -2218,12 +2227,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
i915_gem_object_pin_fence(obj);
 
dev_priv-mm.interruptible = true;
+   intel_runtime_pm_put(dev_priv);
return 0;
 
 err_unpin:
i915_gem_object_unpin_from_display_plane(obj);
 err_interruptible:
dev_priv-mm.interruptible = true;
+   intel_runtime_pm_put(dev_priv);
return ret;
 }
 
@@ -8253,6 +8264,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
*crtc,
 uint32_t width, uint32_t height)
 {
struct drm_device *dev = crtc-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc-pipe;
unsigned old_width, stride;
@@ -8292,6 +8304,15 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
*crtc,
goto fail_locked;
}
 
+   /*
+* Global gtt pte registers are special registers which actually
+* forward writes to a chunk of system memory. Which means that
+* there is no risk that the register values disappear as soon
+* as we call intel_runtime_pm_put(), so it is correct to wrap
+* only the pin/unpin/fence and not more.
+*/
+   intel_runtime_pm_get(dev_priv);
+
/* Note that the w/a also requires 2 PTE of padding following
 * the bo. We currently fill all unused PTE with the shadow
 * page and so we should always have valid PTE following the
@@ -8304,16 +8325,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
*crtc,
ret = i915_gem_object_pin_to_display_plane(obj, alignment, 
NULL);
if (ret) {
DRM_DEBUG_KMS(failed to move cursor bo into the 
GTT\n);
+   intel_runtime_pm_put(dev_priv);
goto fail_locked;
}
 
ret = i915_gem_object_put_fence(obj);
if (ret) {
DRM_DEBUG_KMS(failed to release fence for cursor);
+   intel_runtime_pm_put(dev_priv);
goto 

[Intel-gfx] [PATCH i-g-t 1/1] Add kms_flip_event_leak to .gitignore

2014-08-15 Thread Mason, Michael W
This patch just adds kms_flip_event_leak to tests/.gitignore.

Signed-off-by: Mike Mason michael.w.ma...@intel.com
---
 tests/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/.gitignore b/tests/.gitignore index d14d87b..3da061e 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -121,6 +121,7 @@ kms_cursor_crc
 kms_fbc_crc
 kms_fence_pin_leak
 kms_flip
+kms_flip_event_leak
 kms_flip_tiling
 kms_force_connector
 kms_mmio_vs_cs_flip
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t 1/1] scripts: Allow multiple -t and -x regular expressions for run-tests.sh

2014-08-15 Thread Mason, Michael W
Piglit allows multiple -t and -x regular expressions to be
given on the command line. This patch enables run-tests.sh to
support that as well.

Signed-off-by: Mike Mason michael.w.ma...@intel.com
---
 scripts/run-tests.sh | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh
index cf77473..d0e0c67 100755
--- a/scripts/run-tests.sh
+++ b/scripts/run-tests.sh
@@ -55,8 +55,10 @@ function print_help {
echo   (default: $RESULTS)
echo   -s  create html summary
echo   -t regex  only include tests that match the regular 
expression
+   echo   (can be used more than once)
echo   -v  enable verbose mode
echo   -x regex  exclude tests that match the regular expression
+   echo   (can be used more than once)
echo 
echo Useful patterns for test filtering are described in 
tests/NAMING-CONVENTION
 }
@@ -78,9 +80,9 @@ while getopts :dhlr:st:vx: opt; do
l) list_tests; exit ;;
r) RESULTS=$OPTARG ;;
s) SUMMARY=html ;;
-   t) FILTER=-t $OPTARG ;;
+   t) FILTER=$FILTER -t $OPTARG ;;
v) VERBOSE=-v ;;
-   x) EXCLUDE=-x $OPTARG ;;
+   x) EXCLUDE=$EXCLUDE -x $OPTARG ;;
:)
echo Option -$OPTARG requires an argument.
exit 1
-- 
1.9.1

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


[Intel-gfx] [PATCH i-g-t 1/1] tests: Fix seg fault when gem_mmap is run without specifying a subtest

2014-08-15 Thread Mason, Michael W
From: Mike Mason michael.w.ma...@intel.com

gem_mmap seg faults when all tests are run together. This occurs because
the new-object subtest closes the gem object, but short-mmap assumes
it still exists. Thus gem_mmap__cpu() returns nil for addr and memset()
seg faults. This patch makes new-object and short-mmap create and
close their own gem objects.

Signed-off-by: Mike Mason michael.w.ma...@intel.com
---
 tests/gem_mmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/gem_mmap.c b/tests/gem_mmap.c
index 334bd76..33ffe45 100644
--- a/tests/gem_mmap.c
+++ b/tests/gem_mmap.c
@@ -62,10 +62,8 @@ igt_main
igt_assert(ret == -1  errno == ENOENT);
}

-   igt_fixture
-   handle = gem_create(fd, OBJECT_SIZE);
-
igt_subtest(new-object) {
+   handle = gem_create(fd, OBJECT_SIZE);
arg.handle = handle;
arg.offset = 0;
arg.size = OBJECT_SIZE;
@@ -94,9 +92,11 @@ igt_main

igt_subtest(short-mmap) {
igt_assert(OBJECT_SIZE  4096);
+   handle = gem_create(fd, OBJECT_SIZE);
addr = gem_mmap__cpu(fd, handle, 4096, PROT_WRITE);
memset(addr, 0, 4096);
munmap(addr, 4096);
+   gem_close(fd, handle);
}

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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Initialize workarounds in render ring init fn.

2014-08-15 Thread Michael H. Nguyen



On 08/14/2014 09:51 AM, arun.siluv...@linux.intel.com wrote:

From: Arun Siluvery arun.siluv...@linux.intel.com

The workarounds at the moment are initialized in init_clock_gating() but
they are lost during reset, suspend/resume; this patch moves workarounds
that are part of register state context to render ring init fn otherwise
default context ends up with incorrect values as they don't get initialized
until init_clock_gating fn. This should be ok as they are not related to
display clock gating in the first place.

v2: Add macro to generate w/a table (Daniel)

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_debugfs.c | 38 +++
  drivers/gpu/drm/i915/i915_drv.h | 32 
  drivers/gpu/drm/i915/intel_pm.c | 50 -
  drivers/gpu/drm/i915/intel_ringbuffer.c | 66 +
  4 files changed, 136 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e737b7..fc28835 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2406,6 +2406,43 @@ static int i915_shared_dplls_info(struct seq_file *m, 
void *unused)
return 0;
  }

+static int intel_wa_registers(struct seq_file *m, void *unused)
+{
+   struct drm_info_node *node = (struct drm_info_node *) m-private;
+   struct drm_device *dev = node-minor-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int i;
+   int ret;
+
+   if (!dev_priv-num_wa) {
+   DRM_DEBUG_DRIVER(Workaround table not available\n);
+   return -EINVAL;
+   }
+
+   if (dev_priv-num_wa  I915_MAX_WA_REGS)
+   dev_priv-num_wa = I915_MAX_WA_REGS;
+
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+
+   intel_runtime_pm_get(dev_priv);
+
+   seq_printf(m, Workarounds applied: %d\n, dev_priv-num_wa);
+   for (i = 0; i  dev_priv-num_wa; ++i) {
+   if (dev_priv-wa_regs[i].addr)
+   seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n,
+  dev_priv-wa_regs[i].addr,
+  I915_READ(dev_priv-wa_regs[i].addr),
+  dev_priv-wa_regs[i].mask);
+   }
+
+   intel_runtime_pm_put(dev_priv);
+   mutex_unlock(dev-struct_mutex);
+
+   return 0;
+}
+
  struct pipe_crc_info {
const char *name;
struct drm_device *dev;
@@ -3936,6 +3973,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{i915_semaphore_status, i915_semaphore_status, 0},
{i915_shared_dplls_info, i915_shared_dplls_info, 0},
{i915_dp_mst_info, i915_dp_mst_info, 0},
+   {intel_wa_registers, intel_wa_registers, 0},
  };
  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18c9ad8..c4190a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1222,6 +1222,24 @@ struct i915_gpu_error {
unsigned int test_irq_rings;
  };

+/*
+ * workarounds are currently applied at different places and changes
+ * are being done to consolidate them at a single place hence the
+ * exact count is not clear at this point; replace this with accurate
+ * number based on gen later.
+ */
+#define I915_MAX_WA_REGS  16
+
+/*
+ * structure to verify workarounds status
+ * mask: represent w/a bits
+ */
+struct intel_wa {
+   u32 addr;
+   u32 val;
+   u32 mask;
+};


Can we rename this to intel_reg_wa or similar? intel_wa confused me for 
a second. Personally when I think about wa's, they can be simple 
register writes like is the case here or more complex requiring some 
software logic.


Also, should this be defined in a userspace accessible header? I see 
this defined again locally in the IGT test.



+
  enum modeset_restore {
MODESET_ON_LID_OPEN,
MODESET_DONE,
@@ -1422,6 +1440,9 @@ struct drm_i915_private {
struct drm_i915_gem_object *semaphore_obj;
uint32_t last_seqno, next_seqno;

+   uint32_t num_wa;
+   struct intel_wa wa_regs[I915_MAX_WA_REGS];
+
drm_dma_handle_t *status_page_dmah;
struct resource mch_res;

@@ -2803,6 +2824,17 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, 
int val);
  #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg)
  #define POSTING_READ16(reg)   (void)I915_READ16_NOTRACE(reg)

+#define I915_WRITE_WA(reg, val) ({ \
+   if (dev_priv-num_wa = 0 \
+dev_priv-num_wa  I915_MAX_WA_REGS) { \
+   dev_priv-wa_regs[dev_priv-num_wa].addr = reg;   \
+   dev_priv-wa_regs[dev_priv-num_wa].mask  \
+   =