Re: [Intel-gfx] Responsiveness Changes to i915 Driver
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
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
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
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
-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
-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
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
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
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
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
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
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
-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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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 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
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
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
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 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
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()
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
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
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
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
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()
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
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()
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
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
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
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
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.
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 \ + =