[Intel-gfx] [PATCH] drm: Include task-name and master status in debugfs clients info
Showing who is the current master is useful for trying to decypher errors when trying to acquire master (e.g. a race with X taking over from plymouth). By including the process name as well as the pid simplifies the task of grabbing enough information remotely at the point of error. v2: Add the command column header and flesh out a couple of comments. (David Herrmann) Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Herrmann dh.herrm...@gmail.com Reviewed-by: David Herrmann dh.herrm...@gmail.com --- drivers/gpu/drm/drm_info.c | 27 ++- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c index 15ec9f471ece..36aba980ea8b 100644 --- a/drivers/gpu/drm/drm_info.c +++ b/drivers/gpu/drm/drm_info.c @@ -183,15 +183,32 @@ int drm_clients_info(struct seq_file *m, void *data) struct drm_device *dev = node-minor-dev; struct drm_file *priv; + seq_printf(m, + %20s %5s %3s master a %5s %10s\n, + command, + pid, + dev, + uid, + magic); + + /* dev-filelist is sorted youngest first, but we want to present +* oldest first (i.e. kernel, servers, clients), so walk backwardss. +*/ mutex_lock(dev-struct_mutex); - seq_printf(m, a devpiduid magic\n\n); - list_for_each_entry(priv, dev-filelist, lhead) { - seq_printf(m, %c %3d %5d %5d %10u\n, - priv-authenticated ? 'y' : 'n', - priv-minor-index, + list_for_each_entry_reverse(priv, dev-filelist, lhead) { + struct task_struct *task; + + rcu_read_lock(); /* locks pid_task()-comm */ + task = pid_task(priv-pid, PIDTYPE_PID); + seq_printf(m, %20s %5d %3d %c%c %5d %10u\n, + task ? task-comm : unknown, pid_vnr(priv-pid), + priv-minor-index, + priv-is_master ? 'y' : 'n', + priv-authenticated ? 'y' : 'n', from_kuid_munged(seq_user_ns(m), priv-uid), priv-magic); + rcu_read_unlock(); } mutex_unlock(dev-struct_mutex); return 0; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't call intel_plane_restore() when the prop value didn't change
On Mon, Sep 01, 2014 at 04:16:59PM +0100, Damien Lespiau wrote: Cc: Thomas Wood thomas.w...@intel.com Cc: Sonika Jindal sonika.jin...@intel.com Tested-by: Mika Kuoppala mika.kuopp...@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Damien Lespiau damien.lesp...@intel.com This is also: Tested-by: Alan Stern st...@rowland.harvard.edu -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Prevent recursive deadlock on releasing a busy userptr
During release of the GEM object we hold the struct_mutex. As the object may be holding onto the last reference for the task-mm, calling mmput() may trigger exit_mmap() which close the vma which will call drm_gem_vm_close() and attempt to reacquire the struct_mutex. In order to avoid that recursion, we have to defer the mmput() until after we drop the struct_mutex, i.e. we need to schedule a worker to do the clean up. A further issue spotted by Tvrtko was caused when we took a GTT mmapping of a userptr buffer object. In that case, we would never call mmput as the object would be cyclically referenced by the GTT mmapping and not freed upon process exit - keeping the entire process mm alive after the process task was reaped. The fix employed is to replace the mm_users/mmput() reference handling to mm_count/mmdrop() for the shared i915_mm_struct. INFO: task test_surfaces:1632 blocked for more than 120 seconds. Tainted: GF O 3.14.5+ #1 echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. test_surfaces D 0 1632 1590 0x0082 88014914baa8 0046 88014914a010 00012c40 00012c40 8800a0058210 88014784b010 88014914a010 880037b1c820 8800a0058210 880037b1c824 Call Trace: [81582499] schedule+0x29/0x70 [815825fe] schedule_preempt_disabled+0xe/0x10 [81583b93] __mutex_lock_slowpath+0x183/0x220 [81583c53] mutex_lock+0x23/0x40 [a005c2a3] drm_gem_vm_close+0x33/0x70 [drm] [8115a483] remove_vma+0x33/0x70 [8115a5dc] exit_mmap+0x11c/0x170 [8104d6eb] mmput+0x6b/0x100 [a00f44b9] i915_gem_userptr_release+0x89/0xc0 [i915] [a00e6706] i915_gem_free_object+0x126/0x250 [i915] [a005c06a] drm_gem_object_free+0x2a/0x40 [drm] [a005cc32] drm_gem_object_handle_unreference_unlocked+0xe2/0x120 [drm] [a005ccd4] drm_gem_object_release_handle+0x64/0x90 [drm] [8127ffeb] idr_for_each+0xab/0x100 [a005cc70] ? drm_gem_object_handle_unreference_unlocked+0x120/0x120 [drm] [81583c46] ? mutex_lock+0x16/0x40 [a005c354] drm_gem_release+0x24/0x40 [drm] [a005b82b] drm_release+0x3fb/0x480 [drm] [8118d482] __fput+0xb2/0x260 [8118d6de] fput+0xe/0x10 [8106f27f] task_work_run+0x8f/0xf0 [81052228] do_exit+0x1a8/0x480 [81052551] do_group_exit+0x51/0xc0 [810525d7] SyS_exit_group+0x17/0x20 [8158e092] system_call_fastpath+0x16/0x1b v2: Incorporate feedback from Tvrtko and remove the unnessary mm referencing when creating the i915_mm_struct and improve some of the function names and comments. Reported-by: Jacek Danecki jacek.dane...@intel.com Test-case: igt/gem_userptr_blits/process-exit* Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Tested-by: Gong, Zhipeng zhipeng.g...@intel.com Cc: Jacek Danecki jacek.dane...@intel.com Cc: Ursulin, Tvrtko tvrtko.ursu...@intel.com Reviewed-by: Ursulin, Tvrtko tvrtko.ursu...@intel.com Tested-by: Wang, Wendy wendy.w...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 12 +- drivers/gpu/drm/i915/i915_gem.c | 7 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 412 ++-- 3 files changed, 238 insertions(+), 193 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4aa540348f24..a9d28b7c64ee 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -189,6 +189,7 @@ enum hpd_pin { if ((1 (domain)) (mask)) struct drm_i915_private; +struct i915_mm_struct; struct i915_mmu_object; struct i915_gem_request; @@ -1545,9 +1546,8 @@ struct drm_i915_private { struct i915_gtt gtt; /* VM representing the global address space */ struct i915_gem_mm mm; -#if defined(CONFIG_MMU_NOTIFIER) - DECLARE_HASHTABLE(mmu_notifiers, 7); -#endif + DECLARE_HASHTABLE(mm_structs, 7); + struct mutex mm_lock; /* Kernel Modesetting */ @@ -1852,8 +1852,8 @@ struct drm_i915_gem_object { unsigned workers :4; #define I915_GEM_USERPTR_MAX_WORKERS 15 - struct mm_struct *mm; - struct i915_mmu_object *mn; + struct i915_mm_struct *mm; + struct i915_mmu_object *mmu_object; struct work_struct *work; } userptr; }; @@ -2269,7 +2269,7 @@ int i915_gem_set_tiling(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling(struct drm_device *dev, void *data, struct drm_file *file_priv); -int i915_gem_init_userptr(struct drm_device *dev); +void i915_gem_init_userptr(struct drm_device *dev); int
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? J. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- 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 v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Tue, Sep 02, 2014 at 10:41:05AM +0300, Jani Nikula wrote: On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? I don't know anymore. Just kill the WARN_ON() if it makes things confusing? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/hdmi: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the EDID found during output discovery and reuse it. This is a separate issue from caching the output EDID across detection cycles. v2: Also hookup the force() callback for audio detection when the user forces the connection status. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 145 +- 1 file changed, 80 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 9169786dbbc3..7428521d2e25 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -962,104 +962,117 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, return true; } -static enum drm_connector_status -intel_hdmi_detect(struct drm_connector *connector, bool force) +static void +intel_hdmi_unset_edid(struct drm_connector *connector) { - struct drm_device *dev = connector-dev; struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); - struct intel_digital_port *intel_dig_port = - hdmi_to_dig_port(intel_hdmi); - struct intel_encoder *intel_encoder = intel_dig_port-base; - struct drm_i915_private *dev_priv = dev-dev_private; - struct edid *edid; - enum intel_display_power_domain power_domain; - enum drm_connector_status status = connector_status_disconnected; - DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, - connector-base.id, connector-name); + intel_hdmi-has_hdmi_sink = false; + intel_hdmi-has_audio = false; + intel_hdmi-rgb_quant_range_selectable = false; + + kfree(to_intel_connector(connector)-detect_edid); + to_intel_connector(connector)-detect_edid = NULL; +} + +static bool +intel_hdmi_set_edid(struct drm_connector *connector) +{ + struct drm_i915_private *dev_priv = to_i915(connector-dev); + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + struct intel_encoder *intel_encoder = + hdmi_to_dig_port(intel_hdmi)-base; + enum intel_display_power_domain power_domain; + struct edid *edid; + bool connected = false; power_domain = intel_display_port_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); - intel_hdmi-has_hdmi_sink = false; - intel_hdmi-has_audio = false; - intel_hdmi-rgb_quant_range_selectable = false; edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, intel_hdmi-ddc_bus)); - if (edid) { - if (edid-input DRM_EDID_INPUT_DIGITAL) { - status = connector_status_connected; - if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) - intel_hdmi-has_hdmi_sink = - drm_detect_hdmi_monitor(edid); - intel_hdmi-has_audio = drm_detect_monitor_audio(edid); - intel_hdmi-rgb_quant_range_selectable = - drm_rgb_quant_range_selectable(edid); - } - kfree(edid); - } + intel_display_power_put(dev_priv, power_domain); + + to_intel_connector(connector)-detect_edid = edid; + if (edid edid-input DRM_EDID_INPUT_DIGITAL) { + intel_hdmi-rgb_quant_range_selectable = + drm_rgb_quant_range_selectable(edid); - if (status == connector_status_connected) { + intel_hdmi-has_audio = drm_detect_monitor_audio(edid); if (intel_hdmi-force_audio != HDMI_AUDIO_AUTO) intel_hdmi-has_audio = - (intel_hdmi-force_audio == HDMI_AUDIO_ON); - intel_encoder-type = INTEL_OUTPUT_HDMI; + intel_hdmi-force_audio == HDMI_AUDIO_ON; + + if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) + intel_hdmi-has_hdmi_sink = + drm_detect_hdmi_monitor(edid); + + connected = true; } - intel_display_power_put(dev_priv, power_domain); + return connected; +} + +static enum drm_connector_status +intel_hdmi_detect(struct drm_connector *connector, bool force) +{ + enum drm_connector_status status; + + DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, + connector-base.id, connector-name); + + intel_hdmi_unset_edid(connector); + + if (intel_hdmi_set_edid(connector)) { + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + + hdmi_to_dig_port(intel_hdmi)-base.type = INTEL_OUTPUT_HDMI; + status = connector_status_connected; + } else +
[Intel-gfx] [PATCH 1/3] drm/i915/dp: Refactor common eDP lid detection
Both gmch and pch detection routines used the exact same routine for eDP, so de-duplicate. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_dp.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 048eb2bec04c..42de5cc86b0d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3725,20 +3725,24 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) } static enum drm_connector_status +edp_detect(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + enum drm_connector_status status; + + status = intel_panel_detect(dev); + if (status == connector_status_unknown) + status = connector_status_connected; + + return status; +} + +static enum drm_connector_status ironlake_dp_detect(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); - enum drm_connector_status status; - - /* Can't disconnect eDP, but you can close the lid... */ - if (is_edp(intel_dp)) { - status = intel_panel_detect(dev); - if (status == connector_status_unknown) - status = connector_status_connected; - return status; - } if (!ibx_digital_port_connected(dev_priv, intel_dig_port)) return connector_status_disconnected; @@ -3794,16 +3798,6 @@ g4x_dp_detect(struct intel_dp *intel_dp) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); int ret; - /* Can't disconnect eDP, but you can close the lid... */ - if (is_edp(intel_dp)) { - enum drm_connector_status status; - - status = intel_panel_detect(dev); - if (status == connector_status_unknown) - status = connector_status_connected; - return status; - } - ret = g4x_digital_port_connected(dev, intel_dig_port); if (ret == -EINVAL) return connector_status_unknown; @@ -3877,11 +3871,13 @@ intel_dp_detect(struct drm_connector *connector, bool force) intel_dp-has_audio = false; - if (HAS_PCH_SPLIT(dev)) + /* Can't disconnect eDP, but you can close the lid... */ + if (is_edp(intel_dp)) + status = edp_detect(intel_dp); + else if (HAS_PCH_SPLIT(dev)) status = ironlake_dp_detect(intel_dp); else status = g4x_dp_detect(intel_dp); - if (status != connector_status_connected) goto out; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/dp: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the EDID found during output discovery and reuse it. This is a separate issue from caching the output EDID across detection cycles. v2: Implement connector-force() callback so that edid is associated with the connector for user overrides as well (Ville) Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 156 ++- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 90 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 42de5cc86b0d..69ff82a7ec4c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3808,9 +3808,9 @@ g4x_dp_detect(struct intel_dp *intel_dp) } static struct edid * -intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) +intel_dp_get_edid(struct intel_dp *intel_dp) { - struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_connector *intel_connector = intel_dp-attached_connector; /* use cached edid if we have one */ if (intel_connector-edid) { @@ -3819,27 +3819,55 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) return NULL; return drm_edid_duplicate(intel_connector-edid); - } + } else + return drm_get_edid(intel_connector-base, + intel_dp-aux.ddc); +} - return drm_get_edid(connector, adapter); +static void +intel_dp_set_edid(struct intel_dp *intel_dp) +{ + struct intel_connector *intel_connector = intel_dp-attached_connector; + struct edid *edid; + + edid = intel_dp_get_edid(intel_dp); + intel_connector-detect_edid = edid; + + if (intel_dp-force_audio != HDMI_AUDIO_AUTO) + intel_dp-has_audio = intel_dp-force_audio == HDMI_AUDIO_ON; + else + intel_dp-has_audio = drm_detect_monitor_audio(edid); } -static int -intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter) +static void +intel_dp_unset_edid(struct intel_dp *intel_dp) { - struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_connector *intel_connector = intel_dp-attached_connector; - /* use cached edid if we have one */ - if (intel_connector-edid) { - /* invalid edid */ - if (IS_ERR(intel_connector-edid)) - return 0; + kfree(intel_connector-detect_edid); + intel_connector-detect_edid = NULL; - return intel_connector_update_modes(connector, - intel_connector-edid); - } + intel_dp-has_audio = false; +} - return intel_ddc_get_modes(connector, adapter); +static enum intel_display_power_domain +intel_dp_power_get(struct intel_dp *dp) +{ + struct intel_encoder *encoder = dp_to_dig_port(dp)-base; + enum intel_display_power_domain power_domain; + + power_domain = intel_display_port_power_domain(encoder); + intel_display_power_get(to_i915(encoder-base.dev), power_domain); + + return power_domain; +} + +static void +intel_dp_power_put(struct intel_dp *dp, + enum intel_display_power_domain power_domain) +{ + struct intel_encoder *encoder = dp_to_dig_port(dp)-base; + intel_display_power_put(to_i915(encoder-base.dev), power_domain); } static enum drm_connector_status @@ -3849,27 +3877,22 @@ intel_dp_detect(struct drm_connector *connector, bool force) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct intel_encoder *intel_encoder = intel_dig_port-base; struct drm_device *dev = connector-dev; - struct drm_i915_private *dev_priv = dev-dev_private; enum drm_connector_status status; enum intel_display_power_domain power_domain; - struct edid *edid = NULL; bool ret; - power_domain = intel_display_port_power_domain(intel_encoder); - intel_display_power_get(dev_priv, power_domain); - DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, connector-base.id, connector-name); + intel_dp_unset_edid(intel_dp); if (intel_dp-is_mst) { /* MST devices are disconnected from a monitor POV */ if (intel_encoder-type != INTEL_OUTPUT_EDP) intel_encoder-type = INTEL_OUTPUT_DISPLAYPORT; - status = connector_status_disconnected; - goto out; + return connector_status_disconnected; } - intel_dp-has_audio = false; + power_domain = intel_dp_power_get(intel_dp); /* Can't disconnect eDP, but you can close the lid... */ if
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Tue, Sep 02, 2014 at 11:16:12AM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 10:41:05AM +0300, Jani Nikula wrote: On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? I don't know anymore. Just kill the WARN_ON() if it makes things confusing? Just drop the WARN_ON. I prefer the if() using the status rather than type, as that seems more idiomatic (when looking at our other detection routines). -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 v2] drm/i915: Rework workaround init functions for BDW and CHV
This rework is based on suggestion from Chris. Now w/a are organized in an array and all of them are emitted in single fn instead of sending them individually. This approach is clean and new w/a can be added with minimal changes. The same array can be used when exporting them to debugfs and the temporary array in the current implementation is not required. v2: As suggested by Damien a new w/a reg definition is added. This helps in initializing w/a that are unmasked registers. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 158 ++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 26 ++ 2 files changed, 93 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c5e4dc7..3ed0ad5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -650,87 +650,80 @@ err: return ret; } -static inline void intel_ring_emit_wa(struct intel_engine_cs *ring, - u32 addr, u32 value) +static int i915_request_emit_lri(struct intel_engine_cs *ring, +int num_registers, +const struct intel_wa_reg *lri_list) { - struct drm_device *dev = ring-dev; - struct drm_i915_private *dev_priv = dev-dev_private; + int i; + int ret; + struct drm_i915_private *dev_priv = ring-dev-dev_private; - if (dev_priv-num_wa_regs I915_MAX_WA_REGS) - return; + ret = intel_ring_begin(ring, (2 * num_registers + 1)); + if (ret) + return ret; - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); - intel_ring_emit(ring, addr); - intel_ring_emit(ring, value); + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_registers)); + for (i = 0; i num_registers; ++i) { + u32 value; + const struct intel_wa_reg *p = (lri_list + i); - dev_priv-intel_wa_regs[dev_priv-num_wa_regs].addr = addr; - dev_priv-intel_wa_regs[dev_priv-num_wa_regs].mask = (value) 0x; - /* value is updated with the status of remaining bits of this -* register when it is read from debugfs file -*/ - dev_priv-intel_wa_regs[dev_priv-num_wa_regs].value = value; - dev_priv-num_wa_regs++; + if (p-masked_register) + value = (p-mask 16) | p-value; + else + value = (I915_READ(p-addr) ~p-mask) | p-value; + intel_ring_emit(ring, p-addr); + intel_ring_emit(ring, value); + } - return; + intel_ring_emit(ring, MI_NOOP); + intel_ring_advance(ring); + + return 0; } -static int bdw8_init_workarounds(struct intel_engine_cs *ring) -{ - int ret; - struct drm_device *dev = ring-dev; - struct drm_i915_private *dev_priv = dev-dev_private; +static const struct intel_wa_reg bdw_ring_init_context[] = { /* * workarounds applied in this fn are part of register state context, * they need to be re-initialized followed by gpu reset, suspend/resume, * module reload. */ - dev_priv-num_wa_regs = 0; - memset(dev_priv-intel_wa_regs, 0, sizeof(dev_priv-intel_wa_regs)); - - /* -* update the number of dwords required based on the -* actual number of workarounds applied -*/ - ret = intel_ring_begin(ring, 24); - if (ret) - return ret; /* WaDisablePartialInstShootdown:bdw */ /* WaDisableThreadStallDopClockGating:bdw */ /* FIXME: Unclear whether we really need this on production bdw. */ - intel_ring_emit_wa(ring, GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE -| STALL_DOP_GATING_DISABLE)); + INIT_MASKED_WA(GEN8_ROW_CHICKEN, + _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE) | + _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)), /* WaDisableDopClockGating:bdw May not be needed for production */ - intel_ring_emit_wa(ring, GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); + INIT_MASKED_WA(GEN7_ROW_CHICKEN2, + _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)), /* * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for * pre-production hardware */ - intel_ring_emit_wa(ring, HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS - | GEN8_SAMPLER_POWER_BYPASS_DIS)); + INIT_MASKED_WA(HALF_SLICE_CHICKEN3, + _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS) | +
[Intel-gfx] [PATCH v2] drm/i915: Rework workaround data exporting to debugfs
Now w/a are organized in an array so we know exactly how many of them are applied; use the same array while exporting data to debugfs and remove the temporary array we currently have in driver priv structure. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 29 + drivers/gpu/drm/i915/i915_drv.h | 14 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +++ drivers/gpu/drm/i915/intel_ringbuffer.h | 8 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2727bda..0c1e294 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2465,6 +2465,14 @@ static int i915_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; + struct intel_ring_context_rodata ro_data; + + ret = ring_context_rodata(dev, ro_data); + if (ret) { + seq_puts(m, Workarounds applied: 0\n); + DRM_DEBUG_DRIVER(Workaround table not available !!\n); + return -EINVAL; + } ret = mutex_lock_interruptible(dev-struct_mutex); if (ret) @@ -2472,18 +2480,15 @@ static int i915_wa_registers(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); - seq_printf(m, Workarounds applied: %d\n, dev_priv-num_wa_regs); - for (i = 0; i dev_priv-num_wa_regs; ++i) { - u32 addr, mask; - - addr = dev_priv-intel_wa_regs[i].addr; - mask = dev_priv-intel_wa_regs[i].mask; - dev_priv-intel_wa_regs[i].value = I915_READ(addr) | mask; - if (dev_priv-intel_wa_regs[i].addr) - seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, - dev_priv-intel_wa_regs[i].addr, - dev_priv-intel_wa_regs[i].value, - dev_priv-intel_wa_regs[i].mask); + seq_printf(m, Workarounds applied: %d\n, ro_data.num_items); + for (i = 0; i ro_data.num_items; ++i) { + u32 addr, mask, value; + + addr = ro_data.init_context[i].addr; + mask = ro_data.init_context[i].mask; + value = ro_data.init_context[i].value; + seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, + addr, value, mask); } intel_runtime_pm_put(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 49b7be7..bcf79f0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1553,20 +1553,6 @@ struct drm_i915_private { struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; - /* -* workarounds are currently applied at different places and -* changes are being done to consolidate them so exact count is -* not clear at this point, use a max value for now. -*/ -#define I915_MAX_WA_REGS 16 - struct { - u32 addr; - u32 value; - /* bitmask representing WA bits */ - u32 mask; - } intel_wa_regs[I915_MAX_WA_REGS]; - u32 num_wa_regs; - /* Reclocking support */ bool render_reclock_avail; bool lvds_downclock_avail; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 3ed0ad5..7262c10 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -769,6 +769,21 @@ static int ring_init_context(struct intel_engine_cs *ring) return ret; } +int ring_context_rodata(struct drm_device *dev, + struct intel_ring_context_rodata *ro_data) +{ + if (IS_CHERRYVIEW(dev)) { + ro_data-init_context = chv_ring_init_context; + ro_data-num_items = ARRAY_SIZE(chv_ring_init_context); + } else if (IS_BROADWELL(dev)) { + ro_data-init_context = bdw_ring_init_context; + ro_data-num_items = ARRAY_SIZE(bdw_ring_init_context); + } else + return -EINVAL; + + return 0; +} + static int init_render_ring(struct intel_engine_cs *ring) { struct drm_device *dev = ring-dev; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index c9ed06c..33454ce 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -343,6 +343,14 @@ struct intel_wa_reg { #define INIT_UNMASKED_WA(_addr, _value) \ INIT_WA_REG(0, _addr, ~(_value), _value) +struct intel_ring_context_rodata { + u32 num_items; +
[Intel-gfx] [PATCH v2] igt/gem_workarounds: rework igt to test workaround registers
kernel patch that exports w/a data to debugfs is reworked so update igt accordingly. Address review comments from Damien. - if kernel is not exposing w/a data instead of failing just skip instead. - if the platform is not exposing w/a table then no of workarounds applied are 0, use this data to skip platform checks. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- tests/gem_workarounds.c | 45 +++-- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c index 32156d2..18ded9a 100644 --- a/tests/gem_workarounds.c +++ b/tests/gem_workarounds.c @@ -62,7 +62,7 @@ int drm_fd; uint32_t devid; static drm_intel_bufmgr *bufmgr; struct intel_batchbuffer *batch; -int num_wa_regs; +int num_wa_regs = 0; struct intel_wa_reg *wa_regs; @@ -152,8 +152,7 @@ static void check_workarounds(enum operation op, int num) igt_info(Address\tbefore\t\tafter\t\tw/a mask\tresult\n); for (i = 0; i num; ++i) { - status = (current_wa[i].value current_wa[i].mask) != - (wa_regs[i].value wa_regs[i].mask); + status = (current_wa[i].value wa_regs[i].mask) != wa_regs[i].value; if (status) ++fail_count; @@ -171,21 +170,15 @@ out: igt_main { igt_fixture { - int i; int fd; int ret; FILE *file; char *line = NULL; size_t line_size; - drm_fd = drm_open_any(); - - bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096); - devid = intel_get_drm_devid(drm_fd); - batch = intel_batchbuffer_alloc(bufmgr, devid); - fd = igt_debugfs_open(i915_wa_registers, O_RDONLY); - igt_assert(fd = 0); + if (fd 0) + igt_skip_on(No Workaround table available !!\n); file = fdopen(fd, r); igt_assert(file 0); @@ -193,32 +186,40 @@ igt_main ret = getline(line, line_size, file); igt_assert(ret 0); sscanf(line, Workarounds applied: %d, num_wa_regs); - igt_assert(num_wa_regs 0); - wa_regs = malloc(num_wa_regs * sizeof(*wa_regs)); + if (num_wa_regs) { + int i = 0; - i = 0; - while(getline(line, line_size, file) 0) { - sscanf(line, 0x%X: 0x%08X, mask: 0x%08X, - wa_regs[i].addr, wa_regs[i].value, - wa_regs[i].mask); - ++i; - } + wa_regs = malloc(num_wa_regs * sizeof(*wa_regs)); + while (getline(line, line_size, file) 0) { + sscanf(line, 0x%X: 0x%08X, mask: 0x%08X, + wa_regs[i].addr, wa_regs[i].value, + wa_regs[i].mask); + ++i; + } + } else + igt_info(No workarounds exported\n); free(line); fclose(file); close(fd); + + drm_fd = drm_open_any(); + + bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096); + devid = intel_get_drm_devid(drm_fd); + batch = intel_batchbuffer_alloc(bufmgr, devid); } igt_subtest(check-workaround-data-after-reset) { - if (IS_BROADWELL(devid)) + if (num_wa_regs) check_workarounds(GPU_RESET, num_wa_regs); else igt_skip_on(No Workaround table available!!\n); } igt_subtest(check-workaround-data-after-suspend-resume) { - if (IS_BROADWELL(devid)) + if (num_wa_regs) check_workarounds(SUSPEND_RESUME, num_wa_regs); else igt_skip_on(No Workaround table available!!\n); -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Prevent recursive deadlock on releasing a busy userptr
On Mon, Sep 01, 2014 at 02:23:17PM +0100, Tvrtko Ursulin wrote: On 08/07/2014 02:20 PM, Chris Wilson wrote: During release of the GEM object we hold the struct_mutex. As the object may be holding onto the last reference for the task-mm, calling mmput() may trigger exit_mmap() which close the vma which will call drm_gem_vm_close() and attempt to reacquire the struct_mutex. In order to avoid that recursion, we have to defer the mmput() until after we drop the struct_mutex, i.e. we need to schedule a worker to do the clean up. A further issue spotted by Tvrtko was caused when we took a GTT mmapping of a userptr buffer object. In that case, we would never call mmput as the object would be cyclically referenced by the GTT mmapping and not freed upon process exit - keeping the entire process mm alive after the process task was reaped. The fix employed is to replace the mm_users/mmput() reference handling to mm_count/mmdrop() for the shared i915_mm_struct. INFO: task test_surfaces:1632 blocked for more than 120 seconds. Tainted: GF O 3.14.5+ #1 echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. test_surfaces D 0 1632 1590 0x0082 88014914baa8 0046 88014914a010 00012c40 00012c40 8800a0058210 88014784b010 88014914a010 880037b1c820 8800a0058210 880037b1c824 Call Trace: [81582499] schedule+0x29/0x70 [815825fe] schedule_preempt_disabled+0xe/0x10 [81583b93] __mutex_lock_slowpath+0x183/0x220 [81583c53] mutex_lock+0x23/0x40 [a005c2a3] drm_gem_vm_close+0x33/0x70 [drm] [8115a483] remove_vma+0x33/0x70 [8115a5dc] exit_mmap+0x11c/0x170 [8104d6eb] mmput+0x6b/0x100 [a00f44b9] i915_gem_userptr_release+0x89/0xc0 [i915] [a00e6706] i915_gem_free_object+0x126/0x250 [i915] [a005c06a] drm_gem_object_free+0x2a/0x40 [drm] [a005cc32] drm_gem_object_handle_unreference_unlocked+0xe2/0x120 [drm] [a005ccd4] drm_gem_object_release_handle+0x64/0x90 [drm] [8127ffeb] idr_for_each+0xab/0x100 [a005cc70] ? drm_gem_object_handle_unreference_unlocked+0x120/0x120 [drm] [81583c46] ? mutex_lock+0x16/0x40 [a005c354] drm_gem_release+0x24/0x40 [drm] [a005b82b] drm_release+0x3fb/0x480 [drm] [8118d482] __fput+0xb2/0x260 [8118d6de] fput+0xe/0x10 [8106f27f] task_work_run+0x8f/0xf0 [81052228] do_exit+0x1a8/0x480 [81052551] do_group_exit+0x51/0xc0 [810525d7] SyS_exit_group+0x17/0x20 [8158e092] system_call_fastpath+0x16/0x1b v2: Incorporate feedback from Tvrtko and remove the unnessary mm referencing when creating the i915_mm_struct and improve some of the function names and comments. [snip] I reviewed this some weeks back and did not spot any issues. Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Queued for -next, thanks for the patch. Aside: Is there other userptr stuff outstanding? I've definitely lost track of them :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Prevent recursive deadlock on releasing a busy userptr
On Tue, Sep 02, 2014 at 11:35:00AM +0200, Daniel Vetter wrote: On Mon, Sep 01, 2014 at 02:23:17PM +0100, Tvrtko Ursulin wrote: On 08/07/2014 02:20 PM, Chris Wilson wrote: During release of the GEM object we hold the struct_mutex. As the object may be holding onto the last reference for the task-mm, calling mmput() may trigger exit_mmap() which close the vma which will call drm_gem_vm_close() and attempt to reacquire the struct_mutex. In order to avoid that recursion, we have to defer the mmput() until after we drop the struct_mutex, i.e. we need to schedule a worker to do the clean up. A further issue spotted by Tvrtko was caused when we took a GTT mmapping of a userptr buffer object. In that case, we would never call mmput as the object would be cyclically referenced by the GTT mmapping and not freed upon process exit - keeping the entire process mm alive after the process task was reaped. The fix employed is to replace the mm_users/mmput() reference handling to mm_count/mmdrop() for the shared i915_mm_struct. INFO: task test_surfaces:1632 blocked for more than 120 seconds. Tainted: GF O 3.14.5+ #1 echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. test_surfaces D 0 1632 1590 0x0082 88014914baa8 0046 88014914a010 00012c40 00012c40 8800a0058210 88014784b010 88014914a010 880037b1c820 8800a0058210 880037b1c824 Call Trace: [81582499] schedule+0x29/0x70 [815825fe] schedule_preempt_disabled+0xe/0x10 [81583b93] __mutex_lock_slowpath+0x183/0x220 [81583c53] mutex_lock+0x23/0x40 [a005c2a3] drm_gem_vm_close+0x33/0x70 [drm] [8115a483] remove_vma+0x33/0x70 [8115a5dc] exit_mmap+0x11c/0x170 [8104d6eb] mmput+0x6b/0x100 [a00f44b9] i915_gem_userptr_release+0x89/0xc0 [i915] [a00e6706] i915_gem_free_object+0x126/0x250 [i915] [a005c06a] drm_gem_object_free+0x2a/0x40 [drm] [a005cc32] drm_gem_object_handle_unreference_unlocked+0xe2/0x120 [drm] [a005ccd4] drm_gem_object_release_handle+0x64/0x90 [drm] [8127ffeb] idr_for_each+0xab/0x100 [a005cc70] ? drm_gem_object_handle_unreference_unlocked+0x120/0x120 [drm] [81583c46] ? mutex_lock+0x16/0x40 [a005c354] drm_gem_release+0x24/0x40 [drm] [a005b82b] drm_release+0x3fb/0x480 [drm] [8118d482] __fput+0xb2/0x260 [8118d6de] fput+0xe/0x10 [8106f27f] task_work_run+0x8f/0xf0 [81052228] do_exit+0x1a8/0x480 [81052551] do_group_exit+0x51/0xc0 [810525d7] SyS_exit_group+0x17/0x20 [8158e092] system_call_fastpath+0x16/0x1b v2: Incorporate feedback from Tvrtko and remove the unnessary mm referencing when creating the i915_mm_struct and improve some of the function names and comments. [snip] I reviewed this some weeks back and did not spot any issues. Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Queued for -next, thanks for the patch. Aside: Is there other userptr stuff outstanding? I've definitely lost track of them :( The one I posted with the extra r-b and t-b has a minor tweak to kill the unused error code during init. but otherwise this is the last outstanding patch. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Rework workaround init functions for BDW and CHV
On Tue, Sep 02, 2014 at 10:14:17AM +0100, Arun Siluvery wrote: This rework is based on suggestion from Chris. Now w/a are organized in an array and all of them are emitted in single fn instead of sending them individually. This approach is clean and new w/a can be added with minimal changes. The same array can be used when exporting them to debugfs and the temporary array in the current implementation is not required. v2: As suggested by Damien a new w/a reg definition is added. This helps in initializing w/a that are unmasked registers. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 158 ++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 26 ++ 2 files changed, 93 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c5e4dc7..3ed0ad5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -650,87 +650,80 @@ err: return ret; } -static inline void intel_ring_emit_wa(struct intel_engine_cs *ring, -u32 addr, u32 value) +static int i915_request_emit_lri(struct intel_engine_cs *ring, + int num_registers, + const struct intel_wa_reg *lri_list) { [...] + for (i = 0; i num_registers; ++i) { + u32 value; + const struct intel_wa_reg *p = (lri_list + i); + if (p-masked_register) + value = (p-mask 16) | p-value; + else + value = (I915_READ(p-addr) ~p-mask) | p-value; There's one case when this won't work, when several WAs for a single 'normal' register are defined. The read done here means only the last of those W/As will end up being applied (because the last LRI to that register will be the value that ends up in the register. We'll probably need to coalesce all W/A defined for a single normal register into one write. Considering that, and the comment about the define, maybe the simplest way to go forward with the patch is to forget the non-masked case for the moment, especially as it's not used in this patch. +#define INIT_MASKED_WA(_addr, _value) \ + INIT_WA_REG(1, _addr, ((_value) 16), ((_value) 0x)) Those outer '()' look unnecessary +#define INIT_UNMASKED_WA(_addr, _value) \ + INIT_WA_REG(0, _addr, ~(_value), _value) + ~value is not a mask. Consider values with one or more bits defined to 0. -- Damien ___ 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 workaround init functions for BDW and CHV
On Tue, Sep 02, 2014 at 10:45:45AM +0100, Damien Lespiau wrote: There's one case when this won't work, when several WAs for a single 'normal' register are defined. The read done here means only the last of those W/As will end up being applied (because the last LRI to that register will be the value that ends up in the register. We'll probably need to coalesce all W/A defined for a single normal register into one write. To more correct/clear, it's not the read that's the problem, in the case where a normal register has several W/A, the last write will override the previous ones. -- Damien ___ 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 workaround init functions for BDW and CHV
On Tue, Sep 02, 2014 at 10:45:45AM +0100, Damien Lespiau wrote: On Tue, Sep 02, 2014 at 10:14:17AM +0100, Arun Siluvery wrote: This rework is based on suggestion from Chris. Now w/a are organized in an array and all of them are emitted in single fn instead of sending them individually. This approach is clean and new w/a can be added with minimal changes. The same array can be used when exporting them to debugfs and the temporary array in the current implementation is not required. v2: As suggested by Damien a new w/a reg definition is added. This helps in initializing w/a that are unmasked registers. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 158 ++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 26 ++ 2 files changed, 93 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c5e4dc7..3ed0ad5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -650,87 +650,80 @@ err: return ret; } -static inline void intel_ring_emit_wa(struct intel_engine_cs *ring, - u32 addr, u32 value) +static int i915_request_emit_lri(struct intel_engine_cs *ring, +int num_registers, +const struct intel_wa_reg *lri_list) { [...] + for (i = 0; i num_registers; ++i) { + u32 value; + const struct intel_wa_reg *p = (lri_list + i); + if (p-masked_register) + value = (p-mask 16) | p-value; + else + value = (I915_READ(p-addr) ~p-mask) | p-value; There's one case when this won't work, when several WAs for a single 'normal' register are defined. The read done here means only the last of those W/As will end up being applied (because the last LRI to that register will be the value that ends up in the register. We'll probably need to coalesce all W/A defined for a single normal register into one write. Considering that, and the comment about the define, maybe the simplest way to go forward with the patch is to forget the non-masked case for the moment, especially as it's not used in this patch. If you are still considering an in-kernel table, despite that it can be done in userspace, design it to track all w/a and not just the ring specific fixups. -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: Rework workaround data exporting to debugfs
On Tue, Sep 02, 2014 at 10:15:31AM +0100, Arun Siluvery wrote: Now w/a are organized in an array so we know exactly how many of them are applied; use the same array while exporting data to debugfs and remove the temporary array we currently have in driver priv structure. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 29 + drivers/gpu/drm/i915/i915_drv.h | 14 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +++ drivers/gpu/drm/i915/intel_ringbuffer.h | 8 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2727bda..0c1e294 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2465,6 +2465,14 @@ static int i915_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; + struct intel_ring_context_rodata ro_data; + + ret = ring_context_rodata(dev, ro_data); + if (ret) { + seq_puts(m, Workarounds applied: 0\n); + DRM_DEBUG_DRIVER(Workaround table not available !!\n); + return -EINVAL; + } ret = mutex_lock_interruptible(dev-struct_mutex); if (ret) @@ -2472,18 +2480,15 @@ static int i915_wa_registers(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); - seq_printf(m, Workarounds applied: %d\n, dev_priv-num_wa_regs); - for (i = 0; i dev_priv-num_wa_regs; ++i) { - u32 addr, mask; - - addr = dev_priv-intel_wa_regs[i].addr; - mask = dev_priv-intel_wa_regs[i].mask; - dev_priv-intel_wa_regs[i].value = I915_READ(addr) | mask; - if (dev_priv-intel_wa_regs[i].addr) - seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, -dev_priv-intel_wa_regs[i].addr, -dev_priv-intel_wa_regs[i].value, -dev_priv-intel_wa_regs[i].mask); + seq_printf(m, Workarounds applied: %d\n, ro_data.num_items); + for (i = 0; i ro_data.num_items; ++i) { + u32 addr, mask, value; + + addr = ro_data.init_context[i].addr; + mask = ro_data.init_context[i].mask; + value = ro_data.init_context[i].value; + seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, +addr, value, mask); } intel_runtime_pm_put(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 49b7be7..bcf79f0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1553,20 +1553,6 @@ struct drm_i915_private { struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; - /* - * workarounds are currently applied at different places and - * changes are being done to consolidate them so exact count is - * not clear at this point, use a max value for now. - */ -#define I915_MAX_WA_REGS 16 - struct { - u32 addr; - u32 value; - /* bitmask representing WA bits */ - u32 mask; - } intel_wa_regs[I915_MAX_WA_REGS]; - u32 num_wa_regs; - /* Reclocking support */ bool render_reclock_avail; bool lvds_downclock_avail; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 3ed0ad5..7262c10 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -769,6 +769,21 @@ static int ring_init_context(struct intel_engine_cs *ring) return ret; } +int ring_context_rodata(struct drm_device *dev, + struct intel_ring_context_rodata *ro_data) +{ + if (IS_CHERRYVIEW(dev)) { + ro_data-init_context = chv_ring_init_context; + ro_data-num_items = ARRAY_SIZE(chv_ring_init_context); + } else if (IS_BROADWELL(dev)) { + ro_data-init_context = bdw_ring_init_context; + ro_data-num_items = ARRAY_SIZE(bdw_ring_init_context); This will kinda break my idea that we could put _all_ static register w/a into these schem, so also everything that's in the various init_clock gating functions. The goal of the test is after all to make sure that we set the w/a bits in the right place, so if you only check the w/a emitted in the context init (and this change here kinda bakes this in) then it will not be really useful. Maybe you should (just as a prove of concept of these refactorings) convert the chv or bdw w/a in the init_clock_gating to this infrastructure too?
[Intel-gfx] [PATCH v3] drm/i915: Fix lock dropping in intel_tv_detect()
From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) v3: Drop WARN_ON(type 0) (Chris) Cc: sta...@vger.kernel.org Cc: Tibor Billes tbil...@gmx.com Reported-by: Tibor Billes tbil...@gmx.com Tested-by: Tibor Billes tbil...@gmx.com Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_tv.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 32186a6..c69d3ce 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1311,6 +1311,7 @@ intel_tv_detect(struct drm_connector *connector, bool force) { struct drm_display_mode mode; struct intel_tv *intel_tv = intel_attached_tv(connector); + enum drm_connector_status status; int type; DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n, @@ -1328,16 +1329,19 @@ intel_tv_detect(struct drm_connector *connector, bool force) if (intel_get_load_detect_pipe(connector, mode, tmp, ctx)) { type = intel_tv_detect_type(intel_tv, connector); intel_release_load_detect_pipe(connector, tmp); + status = type 0 ? + connector_status_disconnected : + connector_status_connected; } else - return connector_status_unknown; + status = connector_status_unknown; drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); } else return connector-status; - if (type 0) - return connector_status_disconnected; + if (status != connector_status_connected) + return status; intel_tv-type = type; intel_tv_find_better_format(connector); -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] igt/gem_workarounds: rework igt to test workaround registers
On Tue, Sep 02, 2014 at 10:18:20AM +0100, Arun Siluvery wrote: - igt_assert(fd = 0); + if (fd 0) + igt_skip_on(No Workaround table available !!\n); That's not quite a correct use of the API. The _on is there to signal the first argument is an expression. This will work only because the string is evaluated to true. You probably want to use igt_skip_on_f() http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html#igt-skip-on-f file = fdopen(fd, r); igt_assert(file 0); @@ -193,32 +186,40 @@ igt_main ret = getline(line, line_size, file); igt_assert(ret 0); sscanf(line, Workarounds applied: %d, num_wa_regs); - igt_assert(num_wa_regs 0); - wa_regs = malloc(num_wa_regs * sizeof(*wa_regs)); + if (num_wa_regs) { + int i = 0; - i = 0; - while(getline(line, line_size, file) 0) { - sscanf(line, 0x%X: 0x%08X, mask: 0x%08X, -wa_regs[i].addr, wa_regs[i].value, -wa_regs[i].mask); - ++i; - } + wa_regs = malloc(num_wa_regs * sizeof(*wa_regs)); + while (getline(line, line_size, file) 0) { + sscanf(line, 0x%X: 0x%08X, mask: 0x%08X, +wa_regs[i].addr, wa_regs[i].value, +wa_regs[i].mask); + ++i; + } + } else + igt_info(No workarounds exported\n); It's a bit weird to just have an igt_info() here and skip in every single subtest after that. How about a: igt_skip_on_f(num_wa_regs == 0, No workarounds exported\n); and continue the rest of the test with the case (num_wa_regs == 0) out of the picture? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: s/seqno/request/ tracking inside objects
Hello, Is this patch going to be split up into more manageable pieces? I tried to apply it to a tree fetched yesterday and got a very large number of conflicts. I don't know whether that is because more execlist patches have been merged or if it is other random changes that have broken it or if I am just missing earlier patches in the set. The patch has been sent with subjects of '[PATCH]', '[PATCH 5/5]' and '[PATCH 3/3]'. However, all three emails seem to be the same humongous single part patch and I can't find any 0/3, 4/5, etc. emails. Am I missing some prep work patches without which the final monster patch is never going to apply? Thanks, John. On 27/08/2014 11:39, Chris Wilson wrote: On Wed, Aug 27, 2014 at 11:55:34AM +0200, Daniel Vetter wrote: On Tue, Aug 12, 2014 at 08:05:51PM +0100, Chris Wilson wrote: At the heart of this change is that the seqno is a too low level of an abstraction to handle the growing complexities of command tracking, both with the introduction of multiple command queues with execbuffer and the potential for reordering with a scheduler. On top of the seqno we have the request. Conceptually this is just a fence, but it also has substantial bookkeeping of its own in order to track the context and batch in flight, for example. It is the central structure upon which we can extend with dependency tracking et al. As regards the objects, they were using the seqno as a simple fence, upon which is check or even wait upon for command completion. This patch exchanges that seqno/ring pair with the request itself. For the majority, lifetime of the request is ordered by how we retire objects then requests. However, both the unlocked waits and probing elsewhere do not tie into the normal request lifetimes and so we need to introduce a kref. Extending the objects to use the request as the fence naturally extends to segregrating read/write fence tracking. This has significance for it reduces the number of semaphores we need to emit, reducing the likelihood of #54226, and improving performance overall. v2: Rebase and split out the othogonal tweaks. A silly happened with this patch. It seemed to nullify our earlier seqno-vs-interrupt w/a. I could not spot why, but gen6+ started to fail with missed interrupts (a good test of our robustness handling). So I ripped out the existing ACTHD read and replaced it with a RING_HEAD to manually check whether the request is complete. That also had the nice consequence of forcing __wait_request() to being the central arbiter of request completion. The keener eyed reviewr will also spot that the reset_counter is moved into the request simplifing __wait_request() callsites and reducing the number of atomic reads by virtue of moving the check for a pending GPU reset to the endpoints of GPU access. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Oscar Mateo oscar.ma...@intel.com Cc: Brad Volkin bradley.d.vol...@intel.com Cc: Kukanova, Svetlana svetlana.kukan...@intel.com So I've tried to split this up and totally failed. Non-complete list of things I didn't manage to untangle: - The mmio flip refactoring. Yeah, that's a fairly instrumental part of the patch. It's not complicated but it does benefit a lot from using requests to both make the decision cleaner and the tracking correct. - The overlay request tracking refactoring. Again, the api changes allow the code to be compacted. - The switch to multiple parallel readers with the resulting cascading changes all over. I thought that was fairly isolated to the gem object. It's glossed over in errors/debugfs for simplicity, which deserves to be fixed given a compact representation of all the requests, and so would kill the icky code to find the last request. - The missed irq w/a prep changes. It's easy to split out the change to re-add the rc6 reference and to ditch the ACT_HEAD read, but the commit message talks about instead reading the RING_HEAD, and I just didn't spot the changes relevant to that in this big diff. Was probably looking in the wrong place. I did mention that I tried that earlier on on the ml, but missed saying that it the forcewake reference didn't unbreak the old w/a in the changelog. - The move_to_active/retire refactoring. There's a pile of code movement in there, but I couldn't spot really what's just refactoring and what is real changed needed for the s/seqno/request/ change. move-to() everything since that now operates on the request. retire(), not a lot changes there, just the extra requests being tracked and the strict lifetime ordering of the reference the object holds onto the requests. - De-duping some of the logical_ring_ functions. Spotted because it conflicted (but was easy to hack around), still this shouldn't really be part of this. Things I've spotted which could be split out but amount to a decent rewrite of the patch: -
Re: [Intel-gfx] [PATCH 3/3] drm/i915/hdmi: Cache EDID for a detection cycle
On Tue, Sep 02, 2014 at 09:24:48AM +0100, Chris Wilson wrote: As we may query the edid multiple times following a detect, record the EDID found during output discovery and reuse it. This is a separate issue from caching the output EDID across detection cycles. v2: Also hookup the force() callback for audio detection when the user forces the connection status. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 145 +- 1 file changed, 80 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 9169786dbbc3..7428521d2e25 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -962,104 +962,117 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, return true; } -static enum drm_connector_status -intel_hdmi_detect(struct drm_connector *connector, bool force) +static void +intel_hdmi_unset_edid(struct drm_connector *connector) { - struct drm_device *dev = connector-dev; struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); - struct intel_digital_port *intel_dig_port = - hdmi_to_dig_port(intel_hdmi); - struct intel_encoder *intel_encoder = intel_dig_port-base; - struct drm_i915_private *dev_priv = dev-dev_private; - struct edid *edid; - enum intel_display_power_domain power_domain; - enum drm_connector_status status = connector_status_disconnected; - DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, - connector-base.id, connector-name); + intel_hdmi-has_hdmi_sink = false; + intel_hdmi-has_audio = false; + intel_hdmi-rgb_quant_range_selectable = false; + + kfree(to_intel_connector(connector)-detect_edid); + to_intel_connector(connector)-detect_edid = NULL; +} + +static bool +intel_hdmi_set_edid(struct drm_connector *connector) +{ + struct drm_i915_private *dev_priv = to_i915(connector-dev); + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + struct intel_encoder *intel_encoder = + hdmi_to_dig_port(intel_hdmi)-base; + enum intel_display_power_domain power_domain; + struct edid *edid; + bool connected = false; power_domain = intel_display_port_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); - intel_hdmi-has_hdmi_sink = false; - intel_hdmi-has_audio = false; - intel_hdmi-rgb_quant_range_selectable = false; edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, intel_hdmi-ddc_bus)); - if (edid) { - if (edid-input DRM_EDID_INPUT_DIGITAL) { - status = connector_status_connected; - if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) - intel_hdmi-has_hdmi_sink = - drm_detect_hdmi_monitor(edid); - intel_hdmi-has_audio = drm_detect_monitor_audio(edid); - intel_hdmi-rgb_quant_range_selectable = - drm_rgb_quant_range_selectable(edid); - } - kfree(edid); - } + intel_display_power_put(dev_priv, power_domain); + + to_intel_connector(connector)-detect_edid = edid; + if (edid edid-input DRM_EDID_INPUT_DIGITAL) { + intel_hdmi-rgb_quant_range_selectable = + drm_rgb_quant_range_selectable(edid); - if (status == connector_status_connected) { + intel_hdmi-has_audio = drm_detect_monitor_audio(edid); if (intel_hdmi-force_audio != HDMI_AUDIO_AUTO) intel_hdmi-has_audio = - (intel_hdmi-force_audio == HDMI_AUDIO_ON); - intel_encoder-type = INTEL_OUTPUT_HDMI; + intel_hdmi-force_audio == HDMI_AUDIO_ON; + + if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) + intel_hdmi-has_hdmi_sink = + drm_detect_hdmi_monitor(edid); I wonder if we should be updating has_hdmi_sink also in intel_hdmi_set_property() when force_audio != HDMI_AUDIO_OFF_DVI. But that's a separate issue so material for another patch. + + connected = true; } - intel_display_power_put(dev_priv, power_domain); + return connected; +} + +static enum drm_connector_status +intel_hdmi_detect(struct drm_connector *connector, bool force) +{ + enum drm_connector_status status; + + DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, + connector-base.id, connector-name); + + intel_hdmi_unset_edid(connector); + + if (intel_hdmi_set_edid(connector)) { +
Re: [Intel-gfx] [PATCH v2] igt/gem_workarounds: rework igt to test workaround registers
On Tue, Sep 02, 2014 at 10:59:06AM +0100, Damien Lespiau wrote: On Tue, Sep 02, 2014 at 10:18:20AM +0100, Arun Siluvery wrote: - igt_assert(fd = 0); + if (fd 0) + igt_skip_on(No Workaround table available !!\n); That's not quite a correct use of the API. The _on is there to signal the first argument is an expression. This will work only because the string is evaluated to true. You probably want to use igt_skip_on_f() http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html#igt-skip-on-f Or just igt_skip_on - we dump the expression that was tested, which should be fairly informational. At least when fd would have a better name ;-) -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] igt/gem_workarounds: rework igt to test workaround registers
On Tue, Sep 02, 2014 at 01:04:34PM +0200, Daniel Vetter wrote: On Tue, Sep 02, 2014 at 10:59:06AM +0100, Damien Lespiau wrote: On Tue, Sep 02, 2014 at 10:18:20AM +0100, Arun Siluvery wrote: - igt_assert(fd = 0); + if (fd 0) + igt_skip_on(No Workaround table available !!\n); That's not quite a correct use of the API. The _on is there to signal the first argument is an expression. This will work only because the string is evaluated to true. You probably want to use igt_skip_on_f() http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html#igt-skip-on-f Or just igt_skip_on - we dump the expression that was tested, which should be fairly informational. At least when fd would have a better name ;-) Use igt_require(), it produces more natural phrasing in usage and output. -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 v3] drm/i915: Fix lock dropping in intel_tv_detect()
On Tue, 02 Sep 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) v3: Drop WARN_ON(type 0) (Chris) Cc: sta...@vger.kernel.org Cc: Tibor Billes tbil...@gmx.com Reported-by: Tibor Billes tbil...@gmx.com Tested-by: Tibor Billes tbil...@gmx.com Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Pushed to drm-intel-fixes, thanks for the patch, review, and testing. BR, Jani. --- drivers/gpu/drm/i915/intel_tv.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 32186a6..c69d3ce 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1311,6 +1311,7 @@ intel_tv_detect(struct drm_connector *connector, bool force) { struct drm_display_mode mode; struct intel_tv *intel_tv = intel_attached_tv(connector); + enum drm_connector_status status; int type; DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n, @@ -1328,16 +1329,19 @@ intel_tv_detect(struct drm_connector *connector, bool force) if (intel_get_load_detect_pipe(connector, mode, tmp, ctx)) { type = intel_tv_detect_type(intel_tv, connector); intel_release_load_detect_pipe(connector, tmp); + status = type 0 ? + connector_status_disconnected : + connector_status_connected; } else - return connector_status_unknown; + status = connector_status_unknown; drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); } else return connector-status; - if (type 0) - return connector_status_disconnected; + if (status != connector_status_connected) + return status; intel_tv-type = type; intel_tv_find_better_format(connector); -- 1.8.5.5 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Don't spam dmesg with rps messages on vlv/chv
From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78e39f8..9bc44f0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3488,17 +3488,18 @@ void valleyview_set_rps(struct drm_device *dev, u8 val) WARN_ON(val dev_priv-rps.max_freq_softlimit); WARN_ON(val dev_priv-rps.min_freq_softlimit); - DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n, -vlv_gpu_freq(dev_priv, dev_priv-rps.cur_freq), -dev_priv-rps.cur_freq, -vlv_gpu_freq(dev_priv, val), val); - if (WARN_ONCE(IS_CHERRYVIEW(dev) (val 1), Odd GPU freq value\n)) val = ~1; - if (val != dev_priv-rps.cur_freq) + if (val != dev_priv-rps.cur_freq) { + DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n, +vlv_gpu_freq(dev_priv, dev_priv-rps.cur_freq), +dev_priv-rps.cur_freq, +vlv_gpu_freq(dev_priv, val), val); + vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); + } I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't spam dmesg with rps messages on vlv/chv
On Tue, 02 Sep 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Oh yes please! Reviewed-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78e39f8..9bc44f0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3488,17 +3488,18 @@ void valleyview_set_rps(struct drm_device *dev, u8 val) WARN_ON(val dev_priv-rps.max_freq_softlimit); WARN_ON(val dev_priv-rps.min_freq_softlimit); - DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n, - vlv_gpu_freq(dev_priv, dev_priv-rps.cur_freq), - dev_priv-rps.cur_freq, - vlv_gpu_freq(dev_priv, val), val); - if (WARN_ONCE(IS_CHERRYVIEW(dev) (val 1), Odd GPU freq value\n)) val = ~1; - if (val != dev_priv-rps.cur_freq) + if (val != dev_priv-rps.cur_freq) { + DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n, + vlv_gpu_freq(dev_priv, dev_priv-rps.cur_freq), + dev_priv-rps.cur_freq, + vlv_gpu_freq(dev_priv, val), val); + vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); + } I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); -- 1.8.5.5 ___ 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] drm/i915: Don't spam dmesg with rps messages on vlv/chv
On Tue, Sep 02, 2014 at 03:12:17PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. I'd be more worried about why. gen6_rps_idle()? -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] drm/i915: Prevent recursive deadlock on releasing a busy userptr
On Tue, 02 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Sep 02, 2014 at 11:35:00AM +0200, Daniel Vetter wrote: On Mon, Sep 01, 2014 at 02:23:17PM +0100, Tvrtko Ursulin wrote: On 08/07/2014 02:20 PM, Chris Wilson wrote: During release of the GEM object we hold the struct_mutex. As the object may be holding onto the last reference for the task-mm, calling mmput() may trigger exit_mmap() which close the vma which will call drm_gem_vm_close() and attempt to reacquire the struct_mutex. In order to avoid that recursion, we have to defer the mmput() until after we drop the struct_mutex, i.e. we need to schedule a worker to do the clean up. A further issue spotted by Tvrtko was caused when we took a GTT mmapping of a userptr buffer object. In that case, we would never call mmput as the object would be cyclically referenced by the GTT mmapping and not freed upon process exit - keeping the entire process mm alive after the process task was reaped. The fix employed is to replace the mm_users/mmput() reference handling to mm_count/mmdrop() for the shared i915_mm_struct. INFO: task test_surfaces:1632 blocked for more than 120 seconds. Tainted: GF O 3.14.5+ #1 echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. test_surfaces D 0 1632 1590 0x0082 88014914baa8 0046 88014914a010 00012c40 00012c40 8800a0058210 88014784b010 88014914a010 880037b1c820 8800a0058210 880037b1c824 Call Trace: [81582499] schedule+0x29/0x70 [815825fe] schedule_preempt_disabled+0xe/0x10 [81583b93] __mutex_lock_slowpath+0x183/0x220 [81583c53] mutex_lock+0x23/0x40 [a005c2a3] drm_gem_vm_close+0x33/0x70 [drm] [8115a483] remove_vma+0x33/0x70 [8115a5dc] exit_mmap+0x11c/0x170 [8104d6eb] mmput+0x6b/0x100 [a00f44b9] i915_gem_userptr_release+0x89/0xc0 [i915] [a00e6706] i915_gem_free_object+0x126/0x250 [i915] [a005c06a] drm_gem_object_free+0x2a/0x40 [drm] [a005cc32] drm_gem_object_handle_unreference_unlocked+0xe2/0x120 [drm] [a005ccd4] drm_gem_object_release_handle+0x64/0x90 [drm] [8127ffeb] idr_for_each+0xab/0x100 [a005cc70] ? drm_gem_object_handle_unreference_unlocked+0x120/0x120 [drm] [81583c46] ? mutex_lock+0x16/0x40 [a005c354] drm_gem_release+0x24/0x40 [drm] [a005b82b] drm_release+0x3fb/0x480 [drm] [8118d482] __fput+0xb2/0x260 [8118d6de] fput+0xe/0x10 [8106f27f] task_work_run+0x8f/0xf0 [81052228] do_exit+0x1a8/0x480 [81052551] do_group_exit+0x51/0xc0 [810525d7] SyS_exit_group+0x17/0x20 [8158e092] system_call_fastpath+0x16/0x1b v2: Incorporate feedback from Tvrtko and remove the unnessary mm referencing when creating the i915_mm_struct and improve some of the function names and comments. [snip] I reviewed this some weeks back and did not spot any issues. Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Queued for -next, thanks for the patch. Aside: Is there other userptr stuff outstanding? I've definitely lost track of them :( The one I posted with the extra r-b and t-b has a minor tweak to kill the unused error code during init. but otherwise this is the last outstanding patch. This was referenced from [1], do we need a stable backport? BR, Jani. [1] https://bugs.freedesktop.org/show_bug.cgi?id=80745 -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 -- 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 v2 09/14] drm/i915: Fix edp vdd locking
On Tue, 2014-08-19 at 20:32 +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Introduce a new mutex (pps_mutex) to protect the power sequencer state. For now this state includes want_panel_vdd as well as the power sequencer registers. We need a single mutex (as opposed to per port) because later on we will need to deal with VLV/CHV which have multiple power sequencer which can be reassigned to different ports. v2: Add the locking to intel_dp_encoder_suspend too (Imre) Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_display.c | 2 + drivers/gpu/drm/i915/intel_dp.c | 99 3 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6fbd316..c5faefb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1501,6 +1501,9 @@ struct drm_i915_private { /* LVDS info */ bool no_aux_handshake; + /* protects panel power sequencer state */ + struct mutex pps_mutex; + struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */ int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */ int num_fence_regs; /* 8 on pre-965, 16 otherwise */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0b327eb..ff86729 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12437,6 +12437,8 @@ static void intel_init_display(struct drm_device *dev) } intel_panel_init_backlight_funcs(dev); + + mutex_init(dev_priv-pps_mutex); } /* diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9efa6bf..446df28 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -300,6 +300,8 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) enum port port = intel_dig_port-port; enum pipe pipe; + lockdep_assert_held(dev_priv-pps_mutex); + /* modeset should have pipe */ if (crtc) return to_intel_crtc(crtc)-pipe; @@ -352,6 +354,8 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART) return 0; + mutex_lock(dev_priv-pps_mutex); + This needs rebasing since 02/14 wasn't applied. pipe = vlv_power_sequencer_pipe(intel_dp); pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe); @@ -364,6 +368,8 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); msleep(intel_dp-panel_power_cycle_delay); + mutex_unlock(dev_priv-pps_mutex); + return 0; } @@ -372,6 +378,8 @@ static bool edp_have_panel_power(struct intel_dp *intel_dp) struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; + lockdep_assert_held(dev_priv-pps_mutex); + return (I915_READ(_pp_stat_reg(intel_dp)) PP_ON) != 0; } @@ -383,6 +391,8 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp) struct intel_encoder *intel_encoder = intel_dig_port-base; enum intel_display_power_domain power_domain; + lockdep_assert_held(dev_priv-pps_mutex); + power_domain = intel_display_port_power_domain(intel_encoder); return intel_display_power_enabled(dev_priv, power_domain) (I915_READ(_pp_ctrl_reg(intel_dp)) EDP_FORCE_VDD) != 0; @@ -533,6 +543,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, bool has_aux_irq = HAS_AUX_IRQ(dev); bool vdd; + mutex_lock(dev_priv-pps_mutex); + /* * We will be called with VDD already enabled for dpcd/edid/oui reads. * In such cases we want to leave VDD enabled and it's up to upper layers @@ -648,6 +660,8 @@ out: if (vdd) edp_panel_vdd_off(intel_dp, false); + mutex_unlock(dev_priv-pps_mutex); + return ret; } @@ -1102,6 +1116,8 @@ static void wait_panel_status(struct intel_dp *intel_dp, struct drm_i915_private *dev_priv = dev-dev_private; u32 pp_stat_reg, pp_ctrl_reg; + lockdep_assert_held(dev_priv-pps_mutex); + pp_stat_reg = _pp_stat_reg(intel_dp); pp_ctrl_reg = _pp_ctrl_reg(intel_dp); @@ -1165,6 +1181,8 @@ static u32 ironlake_get_pp_control(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dev-dev_private; u32 control; + lockdep_assert_held(dev_priv-pps_mutex); + control = I915_READ(_pp_ctrl_reg(intel_dp)); control = ~PANEL_UNLOCK_MASK; control |= PANEL_UNLOCK_REGS; @@ -1182,6 +1200,8 @@ static
Re: [Intel-gfx] [PATCH] drm/i915: Don't spam dmesg with rps messages on vlv/chv
On Tue, Sep 02, 2014 at 01:45:29PM +0100, Chris Wilson wrote: On Tue, Sep 02, 2014 at 03:12:17PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. I'd be more worried about why. gen6_rps_idle()? Hmm. Good question. I suppose the PMINTRMSK frobbing should prevent it from doing that. rps_idle should not be involved since it tries to set the max freq all the time when running some benchmark. I'll try to investigate a bit more... -- 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 v2] igt/gem_workarounds: rework igt to test workaround registers
On Tue, Sep 02, 2014 at 12:12:28PM +0100, Chris Wilson wrote: On Tue, Sep 02, 2014 at 01:04:34PM +0200, Daniel Vetter wrote: On Tue, Sep 02, 2014 at 10:59:06AM +0100, Damien Lespiau wrote: On Tue, Sep 02, 2014 at 10:18:20AM +0100, Arun Siluvery wrote: - igt_assert(fd = 0); + if (fd 0) + igt_skip_on(No Workaround table available !!\n); That's not quite a correct use of the API. The _on is there to signal the first argument is an expression. This will work only because the string is evaluated to true. You probably want to use igt_skip_on_f() http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html#igt-skip-on-f Or just igt_skip_on - we dump the expression that was tested, which should be fairly informational. At least when fd would have a better name ;-) Use igt_require(), it produces more natural phrasing in usage and output. Yeah, for new code I second this - we have too many macros ;-) But my practice has been to use igt_skip_on when changing existing code since I seem to be too incompetent to reliably invert boolean conditions ... Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] drm/i915/edp: use max lanes and clock for edp
How about throwing this at any eDP link parameter bugs and regressions? Does it feel too much like giving up the battle? Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d7fc2c0e9ba8..f4248d7f64f9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -889,23 +889,13 @@ intel_dp_compute_config(struct intel_encoder *encoder, bpp = dev_priv-vbt.edp_bpp; } - if (IS_BROADWELL(dev)) { - /* Yes, it's an ugly hack. */ - min_lane_count = max_lane_count; - DRM_DEBUG_KMS(forcing lane count to max (%u) on BDW\n, - min_lane_count); - } else if (dev_priv-vbt.edp_lanes) { - min_lane_count = min(dev_priv-vbt.edp_lanes, -max_lane_count); - DRM_DEBUG_KMS(using min %u lanes per VBT\n, - min_lane_count); - } - - if (dev_priv-vbt.edp_rate) { - min_clock = min(dev_priv-vbt.edp_rate 3, max_clock); - DRM_DEBUG_KMS(using min %02x link bw per VBT\n, - bws[min_clock]); - } + /* +* Use the maximum clock and number of lanes the eDP panel +* advertizes being capable of. Typically these values +* correspond to the native resolution of the panel. +*/ + min_lane_count = max_lane_count; + min_clock = max_clock; } for (; bpp = 6*3; bpp -= 2*3) { -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: debug sink dpms aux errors also on enable
Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d7fc2c0e9ba8..f112d7d2f8bd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1553,8 +1553,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) if (mode != DRM_MODE_DPMS_ON) { ret = drm_dp_dpcd_writeb(intel_dp-aux, DP_SET_POWER, DP_SET_POWER_D3); - if (ret != 1) - DRM_DEBUG_DRIVER(failed to write sink power state\n); } else { /* * When turning on, we need to retry for 1ms to give the sink @@ -1568,6 +1566,10 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) msleep(1); } } + + if (ret != 1) + DRM_DEBUG_KMS(failed to %s sink power state\n, + mode == DRM_MODE_DPMS_ON ? enable : disable); } static bool intel_dp_get_hw_state(struct intel_encoder *encoder, -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 10.1/14] drm/i915: Reset power sequencer pipe tracking when disp2d is off
On Fri, 2014-08-22 at 17:21 +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The power sequencer loses its state when the disp2d power well is down. Clear the dev_priv-pps_pipe tracking so that the power sequencer state gets reinitialized the next time it's needed. v2: Fix the pps_mutex vs. power_domain mutex deadlock by taking power domain reference first Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Imre noticed my previouys attempt was utter garbage. Let's try again. Again maybe we want to squash with 10/14 or at least move the edp_pps_{lock,unlock}() introduction there to avoid churn. The alternative of trying to plumb all the power domain calls out from under pps_mutex seemed a bit too nasty to me, so I opted for this approach instead. drivers/gpu/drm/i915/intel_dp.c | 157 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 + 3 files changed, 106 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d6fe1a2..3468315 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -389,6 +389,67 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) power_seq); } +void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv-dev; + struct intel_encoder *encoder; + + if (WARN_ON(!IS_VALLEYVIEW(dev))) + return; + + /* + * We can't grab pps_mutex here due to deadlock with power_domain + * mutex when power_domain functions are called while holding pps_mutex. + * That also means that in order to use pps_pipe the code needs to + * hold both a power domain reference and pps_mutex, and the power domain + * reference get/put must be done while _not_ holding pps_mutex. + * edp_pps_{lock,unlock}() do these steps in the correct order, so one + * should use them always. + */ + + list_for_each_entry(encoder, dev-mode_config.encoder_list, base.head) { + struct intel_dp *intel_dp; + + if (encoder-type != INTEL_OUTPUT_EDP) + continue; + + intel_dp = enc_to_intel_dp(encoder-base); + intel_dp-pps_pipe = INVALID_PIPE; + } +} + +static void edp_pps_lock(struct intel_dp *intel_dp) +{ + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct intel_encoder *encoder = intel_dig_port-base; + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + enum intel_display_power_domain power_domain; + + /* + * See vlv_power_sequencer_reset() why we need + * a power domain reference here. + */ + power_domain = intel_display_port_power_domain(encoder); + intel_display_power_get(dev_priv, power_domain); + + mutex_lock(dev_priv-pps_mutex); +} + +static void edp_pps_unlock(struct intel_dp *intel_dp) +{ + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct intel_encoder *encoder = intel_dig_port-base; + struct drm_device *dev = encoder-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + enum intel_display_power_domain power_domain; + + mutex_unlock(dev_priv-pps_mutex); + + power_domain = intel_display_port_power_domain(encoder); + intel_display_power_put(dev_priv, power_domain); +} + static u32 _pp_ctrl_reg(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -425,7 +486,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART) return 0; - mutex_lock(dev_priv-pps_mutex); + edp_pps_lock(intel_dp); pipe = vlv_power_sequencer_pipe(intel_dp); @@ -439,7 +500,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); msleep(intel_dp-panel_power_cycle_delay); - mutex_unlock(dev_priv-pps_mutex); + edp_pps_unlock(intel_dp); return 0; } @@ -458,17 +519,13 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); - struct intel_encoder *intel_encoder = intel_dig_port-base; - enum intel_display_power_domain power_domain; lockdep_assert_held(dev_priv-pps_mutex); - power_domain = intel_display_port_power_domain(intel_encoder); - return
Re: [Intel-gfx] [PATCH] drm/i915: Don't spam dmesg with rps messages on vlv/chv
On Tue, Sep 02, 2014 at 04:17:34PM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 01:45:29PM +0100, Chris Wilson wrote: On Tue, Sep 02, 2014 at 03:12:17PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. I'd be more worried about why. gen6_rps_idle()? Hmm. Good question. I suppose the PMINTRMSK frobbing should prevent it from doing that. rps_idle should not be involved since it tries to set the max freq all the time when running some benchmark. I'll try to investigate a bit more... gen6_rps_boost() is one culprit at least. So yeah I think we want this patch, or maybe even make it DRM_DEBUG() to shut it up more? Or maybe just kill it since we don't print this stuff for other platforms either? -- 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: Don't spam dmesg with rps messages on vlv/chv
On Tue, Sep 02, 2014 at 04:46:55PM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 04:17:34PM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 01:45:29PM +0100, Chris Wilson wrote: On Tue, Sep 02, 2014 at 03:12:17PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If the GPU frequency isn't going to change don't spam dmesg with debug messages about it. I'd be more worried about why. gen6_rps_idle()? Hmm. Good question. I suppose the PMINTRMSK frobbing should prevent it from doing that. rps_idle should not be involved since it tries to set the max freq all the time when running some benchmark. I'll try to investigate a bit more... gen6_rps_boost() is one culprit at least. So yeah I think we want this patch, or maybe even make it DRM_DEBUG() to shut it up more? Or maybe just kill it since we don't print this stuff for other platforms either? You were reviewing patches which included a suppression for that, or so I thought this morning Or maybe those patches are only my machines here. Hmm. -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 11/14] drm/i915: Be more careful when picking the initial power sequencer pipe
On Mon, 2014-08-18 at 22:16 +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Try to make sure we find the power sequencer that the BIOS used by first looking for one which has the panel power enabled, then fall back to one with VDD force bit enabled, and finally look at just the port select bits. This should make us pick the correct power sequencer when the BIOS has already enabled the panel. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 36 ++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4614e6e..4952783 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -341,9 +341,31 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) return intel_dp-pps_pipe; } +typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv, +enum pipe pipe); + +static bool vlv_pipe_has_pp_on(struct drm_i915_private *dev_priv, +enum pipe pipe) +{ + return I915_READ(VLV_PIPE_PP_STATUS(pipe)) PP_ON; +} + +static bool vlv_pipe_has_vdd_on(struct drm_i915_private *dev_priv, + enum pipe pipe) +{ + return I915_READ(VLV_PIPE_PP_CONTROL(pipe)) EDP_FORCE_VDD; +} + +static bool vlv_pipe_any(struct drm_i915_private *dev_priv, + enum pipe pipe) +{ + return true; +} + static enum pipe vlv_initial_power_sequencer_pipe(struct drm_i915_private *dev_priv, - enum port port) + enum port port, + vlv_pipe_check pipe_check) { enum pipe pipe; @@ -354,6 +376,9 @@ vlv_initial_power_sequencer_pipe(struct drm_i915_private *dev_priv, if (port_sel != PANEL_PORT_SELECT_VLV(port)) continue; + if (!pipe_check(dev_priv, pipe)) + continue; + return pipe; } @@ -372,7 +397,14 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) lockdep_assert_held(dev_priv-pps_mutex); /* try to find a pipe with this port selected */ - intel_dp-pps_pipe = vlv_initial_power_sequencer_pipe(dev_priv, port); + /* first pick one where the panel is on */ + intel_dp-pps_pipe = vlv_initial_power_sequencer_pipe(dev_priv, port, vlv_pipe_has_pp_on); + /* didn't find one? pick one where vdd is on */ + if (intel_dp-pps_pipe == INVALID_PIPE) + intel_dp-pps_pipe = vlv_initial_power_sequencer_pipe(dev_priv, port, vlv_pipe_has_vdd_on); + /* didn't find one? pick one with just the correct port */ + if (intel_dp-pps_pipe == INVALID_PIPE) + intel_dp-pps_pipe = vlv_initial_power_sequencer_pipe(dev_priv, port, vlv_pipe_any); /* didn't find one? just let vlv_power_sequencer_pipe() pick one when needed */ if (intel_dp-pps_pipe == INVALID_PIPE) { 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
[Intel-gfx] [PATCH 05/10] drm/i915: Relax RPS contraints to allows setting minfreq on idle
When we idle, we set the GPU frequency to minimum. This should be a safety blanket as the pcu on the GPU should be power gating itself anyway. However, in order for us to do set the absolute minimum frequency, we need to relax a few of our assertions that we do not exceed the user limits. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_pm.c | 39 --- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8ea3ff6e062..50af00c5655d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3265,9 +3265,9 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) break; } /* Max/min bins are special */ - if (val == dev_priv-rps.min_freq_softlimit) + if (val = dev_priv-rps.min_freq_softlimit) new_power = LOW_POWER; - if (val == dev_priv-rps.max_freq_softlimit) + if (val = dev_priv-rps.max_freq_softlimit) new_power = HIGH_POWER; if (new_power == dev_priv-rps.power) return; @@ -3365,8 +3365,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val) struct drm_i915_private *dev_priv = dev-dev_private; WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - WARN_ON(val dev_priv-rps.max_freq_softlimit); - WARN_ON(val dev_priv-rps.min_freq_softlimit); + WARN_ON(val dev_priv-rps.max_freq); + WARN_ON(val dev_priv-rps.min_freq); /* min/max delay may still have been modified so be sure to * write the limits value. @@ -3408,10 +3408,11 @@ void gen6_set_rps(struct drm_device *dev, u8 val) static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; + u32 val = dev_priv-rps.min_freq; /* Latest VLV doesn't need to force the gfx clock */ if (dev-pdev-revision = 0xd) { - valleyview_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); + valleyview_set_rps(dev_priv-dev, val); return; } @@ -3419,7 +3420,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) * When we are idle. Drop to min voltage state. */ - if (dev_priv-rps.cur_freq = dev_priv-rps.min_freq_softlimit) + if (dev_priv-rps.cur_freq = val) return; /* Mask turbo interrupt so that they will not come in between */ @@ -3427,10 +3428,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) vlv_force_gfx_clock(dev_priv, true); - dev_priv-rps.cur_freq = dev_priv-rps.min_freq_softlimit; + dev_priv-rps.cur_freq = val; - vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, - dev_priv-rps.min_freq_softlimit); + vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) GENFREQSTATUS) == 0, 5)) @@ -3438,8 +3438,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) vlv_force_gfx_clock(dev_priv, false); - I915_WRITE(GEN6_PMINTRMSK, - gen6_rps_pm_mask(dev_priv, dev_priv-rps.cur_freq)); + I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); } void gen6_rps_idle(struct drm_i915_private *dev_priv) @@ -3448,13 +3447,15 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv) mutex_lock(dev_priv-rps.hw_lock); if (dev_priv-rps.enabled) { + u32 val = dev_priv-rps.min_freq; + if (IS_CHERRYVIEW(dev)) - valleyview_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); + valleyview_set_rps(dev_priv-dev, val); else if (IS_VALLEYVIEW(dev)) vlv_set_rps_idle(dev_priv); - else if (!dev_priv-rps.is_bdw_sw_turbo - || atomic_read(dev_priv-rps.sw_turbo.flip_received)){ - gen6_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); + else if (!dev_priv-rps.is_bdw_sw_turbo || +atomic_read(dev_priv-rps.sw_turbo.flip_received)) { + gen6_set_rps(dev_priv-dev, val); } dev_priv-rps.last_adj = 0; @@ -3499,8 +3500,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val) struct drm_i915_private *dev_priv = dev-dev_private; WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - WARN_ON(val dev_priv-rps.max_freq_softlimit); - WARN_ON(val dev_priv-rps.min_freq_softlimit); + WARN_ON(val dev_priv-rps.max_freq); + WARN_ON(val dev_priv-rps.min_freq); DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n,
[Intel-gfx] [PATCH 07/10] drm/i915: Improved w/a for rps on Baytrail
Rewrite commit 31685c258e0b0ad6aa486c5ec001382cf8a64212 Author: Deepak S deepa...@linux.intel.com Date: Thu Jul 3 17:33:01 2014 -0400 drm/i915/vlv: WA for Turbo and RC6 to work together. Other than code clarity, the major improvement is to disable the extra interrupts generated when idle. However, the reclocking remains rather slow under the new manual regime, in particular it fails to downclock as quickly as desired. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_irq.c | 154 --- drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_display.c | 2 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 13 +++ 5 files changed, 69 insertions(+), 106 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 586625b78cbb..abf9239b58bb 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1268,129 +1268,72 @@ static void notify_ring(struct drm_device *dev, i915_queue_hangcheck(dev); } -static u32 vlv_c0_residency(struct drm_i915_private *dev_priv, - struct intel_rps_ei *rps_ei) +static void vlv_c0_read(struct drm_i915_private *dev_priv, + struct intel_rps_ei *ei) { - u32 cz_ts, cz_freq_khz; - u32 render_count, media_count; - u32 elapsed_render, elapsed_media, elapsed_time; - u32 residency = 0; - - cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP); - cz_freq_khz = DIV_ROUND_CLOSEST(dev_priv-mem_freq * 1000, 4); - - render_count = I915_READ(VLV_RENDER_C0_COUNT_REG); - media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG); - - if (rps_ei-cz_clock == 0) { - rps_ei-cz_clock = cz_ts; - rps_ei-render_c0 = render_count; - rps_ei-media_c0 = media_count; - - return dev_priv-rps.cur_freq; - } - - elapsed_time = cz_ts - rps_ei-cz_clock; - rps_ei-cz_clock = cz_ts; - - elapsed_render = render_count - rps_ei-render_c0; - rps_ei-render_c0 = render_count; - - elapsed_media = media_count - rps_ei-media_c0; - rps_ei-media_c0 = media_count; - - /* Convert all the counters into common unit of milli sec */ - elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC; - elapsed_render /= cz_freq_khz; - elapsed_media /= cz_freq_khz; - - /* -* Calculate overall C0 residency percentage -* only if elapsed time is non zero -*/ - if (elapsed_time) { - residency = - ((max(elapsed_render, elapsed_media) * 100) - / elapsed_time); - } - - return residency; + ei-cz_clock = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP); + ei-render_c0 = I915_READ(VLV_RENDER_C0_COUNT); + ei-media_c0 = I915_READ(VLV_MEDIA_C0_COUNT); } -/** - * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU - * busy-ness calculated from C0 counters of render media power wells - * @dev_priv: DRM device private - * - */ -static int vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv) +static bool vlv_c0_above(struct drm_i915_private *dev_priv, +const struct intel_rps_ei *old, +const struct intel_rps_ei *now, +int threshold) { - u32 residency_C0_up = 0, residency_C0_down = 0; - int new_delay, adj; + u64 time = now-cz_clock - old-cz_clock; + u64 c0 = max(now-render_c0 - old-render_c0, +now-media_c0 - old-media_c0); - dev_priv-rps.ei_interrupt_count++; + c0 *= 100 * VLV_CZ_CLOCK_TO_MILLI_SEC * 4 / 1000; + time *= threshold * dev_priv-mem_freq; + return c0 = time; +} - WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); +void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) +{ + vlv_c0_read(dev_priv, dev_priv-rps.down_ei); + dev_priv-rps.up_ei = dev_priv-rps.down_ei; + dev_priv-rps.ei_interrupt_count = 0; +} +static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) +{ + struct intel_rps_ei now; + u32 events = 0; - if (dev_priv-rps.up_ei.cz_clock == 0) { - vlv_c0_residency(dev_priv, dev_priv-rps.up_ei); - vlv_c0_residency(dev_priv, dev_priv-rps.down_ei); - return dev_priv-rps.cur_freq; - } + if ((pm_iir GEN6_PM_RP_UP_EI_EXPIRED) == 0) + return 0; + vlv_c0_read(dev_priv, now); /* * To down throttle, C0 residency should be less than down threshold * for continous EI intervals. So calculate down EI counters * once
[Intel-gfx] [PATCH 08/10] drm/i915: Use down ei for manual Baytrail RPS calculations
Use both up/down manual ei calcuations for symmetry and greater flexibility for reclocking, instead of faking the down interrupt based on a fixed integer number of up interrupts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/i915_irq.c | 17 +++-- drivers/gpu/drm/i915/i915_reg.h | 1 - drivers/gpu/drm/i915/intel_pm.c | 5 ++--- 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index cefe67fb3949..54d4c8bb7465 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -984,8 +984,6 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; - u32 ei_interrupt_count; - int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index abf9239b58bb..ea127f5dd57d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1294,7 +1294,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) { vlv_c0_read(dev_priv, dev_priv-rps.down_ei); dev_priv-rps.up_ei = dev_priv-rps.down_ei; - dev_priv-rps.ei_interrupt_count = 0; } static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) @@ -1302,21 +1301,11 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) struct intel_rps_ei now; u32 events = 0; - if ((pm_iir GEN6_PM_RP_UP_EI_EXPIRED) == 0) + if ((pm_iir (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED)) == 0) return 0; vlv_c0_read(dev_priv, now); - /* -* To down throttle, C0 residency should be less than down threshold -* for continous EI intervals. So calculate down EI counters -* once in VLV_INT_COUNT_FOR_DOWN_EI -*/ - if (++dev_priv-rps.ei_interrupt_count = VLV_INT_COUNT_FOR_DOWN_EI) { - pm_iir |= GEN6_PM_RP_DOWN_EI_EXPIRED; - dev_priv-rps.ei_interrupt_count = 0; - } - if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, @@ -4607,8 +4596,8 @@ void intel_irq_init(struct drm_device *dev) /* Let's track the enabled rps events */ if (IS_VALLEYVIEW(dev) !IS_CHERRYVIEW(dev)) - /* WaGsvRC0ResidencyMethod:vlv */ - dev_priv-pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; + /* WaGsvRC0ResidenncyMethod:VLV */ + dev_priv-pm_rps_events = GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED; else dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d0f9816ec780..89c532e71af5 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -583,7 +583,6 @@ enum punit_power_well { #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 #define VLV_RP_UP_EI_THRESHOLD 90 #define VLV_RP_DOWN_EI_THRESHOLD 70 -#define VLV_INT_COUNT_FOR_DOWN_EI 5 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a3348da6e8a4..53b488d75882 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3338,11 +3338,10 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val) u32 mask = 0; if (val dev_priv-rps.min_freq_softlimit) - mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; + mask |= GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; if (val dev_priv-rps.max_freq_softlimit) - mask |= GEN6_PM_RP_UP_THRESHOLD; + mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_UP_THRESHOLD; - mask |= dev_priv-pm_rps_events (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED); mask = dev_priv-pm_rps_events; /* IVB and SNB hard hangs on looping batchbuffer -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915: Check for a stalled page flip after each vblank
Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth simon.farnswo...@onelan.co.uk Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. v2: Cleanup, refactor, and rename v3: Only start counting vblanks after the flip command has been seen by the hardware. v4: Record the seqno after we touch the ring, or else there may be no seqno allocated yet. v5: Rebase on mmio-flip. Reported-by: Simon Farnsworth si...@farnz.org.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v4] Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c | 34 +++--- drivers/gpu/drm/i915/i915_irq.c | 84 +++-- drivers/gpu/drm/i915/intel_display.c | 119 --- drivers/gpu/drm/i915/intel_drv.h | 3 + 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 97c257d06729..f7816b4329d6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -518,6 +518,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) { struct drm_info_node *node = m-private; struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; unsigned long flags; struct intel_crtc *crtc; int ret; @@ -537,6 +538,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, No flip due on pipe %c (plane %c)\n, pipe, plane); } else { + u32 addr; + if (atomic_read(work-pending) INTEL_FLIP_COMPLETE) { seq_printf(m, Flip queued on pipe %c (plane %c)\n, pipe, plane); @@ -544,23 +547,34 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, Flip pending (waiting for vsync) on pipe %c (plane %c)\n, pipe, plane); } + if (work-flip_queued_request) { + struct i915_gem_request *rq = work-flip_queued_request; + seq_printf(m, Flip queued on %s at seqno %u, next seqno %u [current breadcrumb %u], completed? %d\n, + rq-engine-name, + rq-seqno, rq-i915-next_seqno, + rq-engine-get_seqno(rq-engine), + __i915_request_complete__wa(rq)); + } else + seq_printf(m, Flip not associated with any ring\n); + seq_printf(m, Flip queued on frame %d, (was ready on frame %d), now %d\n, + work-flip_queued_vblank, + work-flip_ready_vblank, + drm_vblank_count(dev, crtc-pipe)); if (work-enable_stall_check) seq_puts(m, Stall check enabled, ); else seq_puts(m, Stall check waiting for page flip ioctl, ); seq_printf(m, %d prepares\n, atomic_read(work-pending)); - if (work-old_fb_obj) { - struct drm_i915_gem_object *obj =
[Intel-gfx] [PATCH 02/10] drm/i915: Decouple the stuck pageflip on modeset
If we successfully confuse the hardware, and cause it to drop a queued pageflip, we wait for 60s and issue a warning before continuing on with the modeset. However, this leaves the pending pageflip still stuck indefinitely. Pretend to userspace that it does complete, and let us start afresh following the modeset. v2: Rebase after refactor Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 18c431e4f099..bbc3d509bcd7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3434,9 +3434,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) struct drm_i915_private *dev_priv = dev-dev_private; WARN_ON(waitqueue_active(dev_priv-pending_flip_queue)); - WARN_ON(wait_event_timeout(dev_priv-pending_flip_queue, - !intel_crtc_has_pending_flip(crtc), - 60*HZ) == 0); + if (WARN_ON(wait_event_timeout(dev_priv-pending_flip_queue, + !intel_crtc_has_pending_flip(crtc), + 60*HZ) == 0)) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + unsigned long flags; + + spin_lock_irqsave(dev-event_lock, flags); + if (intel_crtc-unpin_work) { + WARN_ONCE(1, Removing stuck page flip\n); + page_flip_completed(intel_crtc); + } + spin_unlock_irqrestore(dev-event_lock, flags); + } if (crtc-primary-fb) { mutex_lock(dev-struct_mutex); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: Agressive downclocking on Baytrail
Reuse the same reclocking strategy for Baytail as on its bigger brethren, Sandybridge and Ivybridge. In particular, this makes the device quicker to reclock (both up and down) though the tendency now is to downclock more aggressively to compensate for the RPS boosts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 11 ++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 54d4c8bb7465..e8e532dcf136 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -984,6 +984,9 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; + u8 up_threshold; /* Current %busy required to uplock */ + u8 down_threshold; /* Current %busy required to downclock */ + int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ea127f5dd57d..9c72d26f1066 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1309,7 +1309,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, - VLV_RP_DOWN_EI_THRESHOLD)) + dev_priv-rps.down_threshold)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv-rps.down_ei = now; } @@ -1317,7 +1317,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, dev_priv-rps.up_ei, now, -VLV_RP_UP_EI_THRESHOLD)) +dev_priv-rps.up_threshold)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv-rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 89c532e71af5..7b3ef6459ed3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -581,8 +581,6 @@ enum punit_power_well { #define FB_FMAX_VMIN_FREQ_LO_MASK0xf800 #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 -#define VLV_RP_UP_EI_THRESHOLD 90 -#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 53b488d75882..70f6d9c392c4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3276,10 +3276,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) switch (new_power) { case LOW_POWER: /* Upclock if more than 95% busy over 16ms */ + dev_priv-rps.up_threshold = 95; I915_WRITE(GEN6_RP_UP_EI, 12500); I915_WRITE(GEN6_RP_UP_THRESHOLD, 11800); /* Downclock if less than 85% busy over 32ms */ + dev_priv-rps.down_threshold = 85; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 21250); @@ -3294,10 +3296,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case BETWEEN: /* Upclock if more than 90% busy over 13ms */ + dev_priv-rps.up_threshold = 90; I915_WRITE(GEN6_RP_UP_EI, 10250); I915_WRITE(GEN6_RP_UP_THRESHOLD, 9225); /* Downclock if less than 75% busy over 32ms */ + dev_priv-rps.down_threshold = 75; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 18750); @@ -3312,10 +3316,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case HIGH_POWER: /* Upclock if more than 85% busy over 10ms */ + dev_priv-rps.up_threshold = 85; I915_WRITE(GEN6_RP_UP_EI, 8000); I915_WRITE(GEN6_RP_UP_THRESHOLD, 6800); /* Downclock if less than 60% busy over 32ms */ + dev_priv-rps.down_threshold = 60; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 15000); @@ -3435,6 +3441,7 @@ static void vlv_set_rps_idle(struct drm_i915_private
[Intel-gfx] [PATCH 03/10] drm/i915: Boost GPU frequency if we detect outstanding pageflips
If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank v4: Rebase v5: Cancel the outstanding work in runtime suspend Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 + drivers/gpu/drm/i915/intel_drv.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 34 ++ 3 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bbc3d509bcd7..455cb0b0dcac 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9761,6 +9761,10 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) intel_crtc-unpin_work-flip_queued_vblank, drm_vblank_count(dev, pipe)); page_flip_completed(intel_crtc); } + if (intel_crtc-unpin_work != NULL + intel_crtc-unpin_work-rcs_active + drm_vblank_count(dev, pipe) - intel_crtc-unpin_work-flip_queued_vblank 1) + intel_queue_rps_boost_for_request(dev, intel_crtc-unpin_work-flip_queued_request); spin_unlock_irqrestore(dev-event_lock, flags); } @@ -9856,6 +9860,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, crtc-primary-fb = fb; work-pending_flip_obj = obj; + work-rcs_active = RCS_ENGINE(dev_priv)-last_request != NULL; atomic_inc(intel_crtc-unpin_work_count); intel_crtc-reset_counter = atomic_read(dev_priv-gpu_error.reset_counter); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1722673c534e..bbd59dd7be1b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -666,6 +666,7 @@ struct intel_unpin_work { int flip_queued_vblank; int flip_ready_vblank; bool enable_stall_check; + bool rcs_active; }; struct intel_set_config { @@ -1076,6 +1077,8 @@ void ironlake_teardown_rc6(struct drm_device *dev); void gen6_update_ring_freq(struct drm_device *dev); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct i915_gem_request *rq); void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2f4e38f36523..a560eb109160 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7568,6 +7568,40 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val) return ret; } +struct request_boost { + struct work_struct work; + struct i915_gem_request *rq; +}; + +static void __intel_rps_boost_work(struct work_struct *work) +{ + struct request_boost *boost = container_of(work, struct request_boost, work); + + if (!i915_request_complete(boost-rq)) + gen6_rps_boost(boost-rq-i915); + + i915_request_put__unlocked(boost-rq); + kfree(boost); +} + +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct i915_gem_request *rq) +{ + struct request_boost *boost; + + if (rq == NULL || INTEL_INFO(dev)-gen 6) + return; + + boost = kmalloc(sizeof(*boost), GFP_ATOMIC); + if (boost == NULL) + return; + + INIT_WORK(boost-work, __intel_rps_boost_work); + boost-rq = i915_request_get(rq); + + queue_work(to_i915(dev)-wq, boost-work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: Deminish contribution of wait-boosting from clients
Only allow each client to perform one RPS boost in each period of GPU activity. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 8 +--- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/i915_gem_request.c | 17 ++--- drivers/gpu/drm/i915/intel_drv.h| 3 ++- drivers/gpu/drm/i915/intel_pm.c | 29 ++--- 5 files changed, 37 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index da33964ce711..cefe67fb3949 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -991,6 +991,7 @@ struct intel_gen6_power_mgmt { bool enabled; struct delayed_work delayed_resume_work; + struct list_head clients; bool is_bdw_sw_turbo; /* Switch of BDW software turbo */ struct intel_rps_bdw_turbo sw_turbo; /* Calculate RP interrupt timing */ @@ -1880,12 +1881,13 @@ struct drm_i915_file_private { struct { spinlock_t lock; struct list_head request_list; - struct delayed_work idle_work; } mm; struct idr context_idr; - atomic_t rps_wait_boost; - struct intel_engine_cs *bsd_engine; + struct list_head rps_boost; + struct intel_engine_cs *bsd_engine; + + unsigned rps_boosts; }; /* diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4d46170f4b74..abbe2c6196cd 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4650,8 +4650,6 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file) { struct drm_i915_file_private *file_priv = file-driver_priv; - cancel_delayed_work_sync(file_priv-mm.idle_work); - /* Clean up our request list when the client is going away, so that * later retire_requests won't dereference our soon-to-be-gone * file_priv. @@ -4667,15 +4665,12 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file) rq-file_priv = NULL; } spin_unlock(file_priv-mm.lock); -} - -static void -i915_gem_file_idle_work_handler(struct work_struct *work) -{ - struct drm_i915_file_private *file_priv = - container_of(work, typeof(*file_priv), mm.idle_work.work); - atomic_set(file_priv-rps_wait_boost, false); + if (!list_empty(file_priv-rps_boost)) { + mutex_lock(to_i915(dev)-rps.hw_lock); + list_del(file_priv-rps_boost); + mutex_unlock(to_i915(dev)-rps.hw_lock); + } } int i915_gem_open(struct drm_device *dev, struct drm_file *file) @@ -4692,11 +4687,10 @@ int i915_gem_open(struct drm_device *dev, struct drm_file *file) file-driver_priv = file_priv; file_priv-dev_priv = dev-dev_private; file_priv-file = file; + INIT_LIST_HEAD(file_priv-rps_boost); spin_lock_init(file_priv-mm.lock); INIT_LIST_HEAD(file_priv-mm.request_list); - INIT_DELAYED_WORK(file_priv-mm.idle_work, - i915_gem_file_idle_work_handler); ret = i915_gem_context_open(dev, file); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 49f93faa0db0..ca777f6c35d7 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -408,14 +408,6 @@ static bool missed_irq(struct i915_gem_request *rq) return test_bit(rq-engine-id, rq-i915-gpu_error.missed_irq_rings); } -static bool can_wait_boost(struct drm_i915_file_private *file_priv) -{ - if (file_priv == NULL) - return true; - - return !atomic_xchg(file_priv-rps_wait_boost, true); -} - bool __i915_request_complete__wa(struct i915_gem_request *rq) { struct drm_i915_private *dev_priv = rq-i915; @@ -475,13 +467,8 @@ int __i915_request_wait(struct i915_gem_request *rq, timeout_expire = timeout_ns ? jiffies + nsecs_to_jiffies((u64)*timeout_ns) : 0; - if (INTEL_INFO(rq-i915)-gen = 6 rq-engine-id == RCS can_wait_boost(file_priv)) { - gen6_rps_boost(rq-i915); - if (file_priv) - mod_delayed_work(rq-i915-wq, -file_priv-mm.idle_work, -msecs_to_jiffies(100)); - } + if (rq-engine-id == RCS INTEL_INFO(rq-i915)-gen = 6) + gen6_rps_boost(rq-i915, file_priv); if (!irq_test_in_progress WARN_ON(!rq-engine-irq_get(rq-engine))) return -ENODEV; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bbd59dd7be1b..36bf92b026a7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1076,7 +1076,8 @@ void intel_reset_gt_powersave(struct
[Intel-gfx] [PATCH 06/10] drm/i915: Rearrange RPS frequency calculation
The goal is to refactor the Cherryview special casing to only be in a single location. To do so we need to compute the desired adjustment and then tweak it for Cherryview. This has the minor side-effect of making sure we set adj to 0 if we directly update the target frequency. Ensuring that the RPS constants are correct for Cherryview is outside the scope of this function Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_irq.c | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 60ca89986499..586625b78cbb 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1420,21 +1420,20 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(dev_priv-rps.hw_lock); adj = dev_priv-rps.last_adj; + new_delay = dev_priv-rps.cur_freq; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? 2 : 1; - } - new_delay = dev_priv-rps.cur_freq + adj; - + else + adj = 1; /* * For better performance, jump directly * to RPe if we're below it. */ - if (new_delay dev_priv-rps.efficient_freq) + if (new_delay dev_priv-rps.efficient_freq - adj) { new_delay = dev_priv-rps.efficient_freq; + adj = 0; + } } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { if (dev_priv-rps.cur_freq dev_priv-rps.efficient_freq) new_delay = dev_priv-rps.efficient_freq; @@ -1446,15 +1445,17 @@ static void gen6_pm_rps_work(struct work_struct *work) } else if (pm_iir GEN6_PM_RP_DOWN_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? -2 : -1; - } - new_delay = dev_priv-rps.cur_freq + adj; + else + adj = -1; } else { /* unknown event */ - new_delay = dev_priv-rps.cur_freq; + adj = 0; } + /* CHV needs even encode values */ + if (IS_CHERRYVIEW(dev_priv-dev)) + adj = 1; + new_delay += adj; + /* sysfs frequency interfaces may have snuck in while servicing the * interrupt */ -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Turn on panel power before doing aux transfers
On Mon, 2014-08-18 at 22:16 +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com On VLV/CHV the panel power sequencer may need to be kicked a bit to lock onto the new port, and that needs to happen before any aux transfers are attempted if we want the aux transfers to actaully succeed. So turn on panel power (part of the kick) before aux transfers (DPMS_ON + link training). This also matches the documented modeset sequence better for pch platforms. The documentation doesn't explicitly state anything about the DPMS or link training DPCD writes, but the panel power on step is always listed before link training is mentioned. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Jani had the same change in: https://bugs.freedesktop.org/show_bug.cgi?id=70117 which solved link training issues, so we could add a reference to that bug and ask the reporters to retest. The patch looks ok: Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4952783..28bc652 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2275,10 +2275,10 @@ static void intel_enable_dp(struct intel_encoder *encoder) return; intel_edp_panel_vdd_on(intel_dp); - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); intel_edp_panel_on(intel_dp); intel_edp_panel_vdd_off(intel_dp, true); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); intel_dp_stop_link_train(intel_dp); } 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
[Intel-gfx] [PATCH 10/10] drm/i915: Move pm_rps_events to i915-rps
Since this mask is only used in conjunction with RPS, move it alongside its brethen in the i915-rps struct. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_irq.c | 35 +-- drivers/gpu/drm/i915/intel_pm.c | 22 +++--- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e8e532dcf136..3461b9838013 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -963,6 +963,7 @@ struct intel_gen6_power_mgmt { /* work and pm_iir are protected by dev_priv-irq_lock */ struct work_struct work; u32 pm_iir; + u32 pm_events; /* Frequencies are stored in potentially platform dependent multiples. * In other words, *_freq needs to be multiplied by X to be interesting. @@ -1486,7 +1487,6 @@ struct drm_i915_private { }; u32 gt_irq_mask; u32 pm_irq_mask; - u32 pm_rps_events; u32 pipestat_irq_mask[I915_MAX_PIPES]; struct delayed_work hotplug_work; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9c72d26f1066..973dd03a21e5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1336,17 +1336,16 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv-rps.pm_iir; dev_priv-rps.pm_iir = 0; if (INTEL_INFO(dev_priv-dev)-gen = 8) - gen8_enable_pm_irq(dev_priv, dev_priv-pm_rps_events); - else { + gen8_enable_pm_irq(dev_priv, dev_priv-rps.pm_events); + else /* Make sure not to corrupt PMIMR state used by ringbuffer */ - gen6_enable_pm_irq(dev_priv, dev_priv-pm_rps_events); - } + gen6_enable_pm_irq(dev_priv, dev_priv-rps.pm_events); spin_unlock_irq(dev_priv-irq_lock); /* Make sure we didn't queue anything we're not going to process. */ - WARN_ON(pm_iir ~dev_priv-pm_rps_events); + WARN_ON(pm_iir ~dev_priv-rps.pm_events); - if ((pm_iir dev_priv-pm_rps_events) == 0) + if ((pm_iir dev_priv-rps.pm_events) == 0) return; mutex_lock(dev_priv-rps.hw_lock); @@ -1549,12 +1548,12 @@ static void snb_gt_irq_handler(struct drm_device *dev, static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) { - if ((pm_iir dev_priv-pm_rps_events) == 0) + if ((pm_iir dev_priv-rps.pm_events) == 0) return; spin_lock(dev_priv-irq_lock); - dev_priv-rps.pm_iir |= pm_iir dev_priv-pm_rps_events; - gen8_disable_pm_irq(dev_priv, pm_iir dev_priv-pm_rps_events); + dev_priv-rps.pm_iir |= pm_iir dev_priv-rps.pm_events; + gen8_disable_pm_irq(dev_priv, pm_iir dev_priv-rps.pm_events); spin_unlock(dev_priv-irq_lock); queue_work(dev_priv-wq, dev_priv-rps.work); @@ -1617,9 +1616,9 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev, if (master_ctl GEN8_GT_PM_IRQ) { tmp = I915_READ(GEN8_GT_IIR(2)); - if (tmp dev_priv-pm_rps_events) { + if (tmp dev_priv-rps.pm_events) { I915_WRITE(GEN8_GT_IIR(2), - tmp dev_priv-pm_rps_events); + tmp dev_priv-rps.pm_events); ret = IRQ_HANDLED; gen8_rps_irq_handler(dev_priv, tmp); } else @@ -1944,10 +1943,10 @@ void gen8_flip_interrupt(struct drm_device *dev) * the work queue. */ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) { - if (pm_iir dev_priv-pm_rps_events) { + if (pm_iir dev_priv-rps.pm_events) { spin_lock(dev_priv-irq_lock); - dev_priv-rps.pm_iir |= pm_iir dev_priv-pm_rps_events; - gen6_disable_pm_irq(dev_priv, pm_iir dev_priv-pm_rps_events); + dev_priv-rps.pm_iir |= pm_iir dev_priv-rps.pm_events; + gen6_disable_pm_irq(dev_priv, pm_iir dev_priv-rps.pm_events); spin_unlock(dev_priv-irq_lock); queue_work(dev_priv-wq, dev_priv-rps.work); @@ -3537,7 +3536,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) GEN5_IRQ_INIT(GT, dev_priv-gt_irq_mask, gt_irqs); if (INTEL_INFO(dev)-gen = 6) { - pm_irqs |= dev_priv-pm_rps_events; + pm_irqs |= dev_priv-rps.pm_events; if (HAS_VEBOX(dev)) pm_irqs |= PM_VEBOX_USER_INTERRUPT; @@ -3742,7 +3741,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv) dev_priv-pm_irq_mask = 0x; GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]); GEN8_IRQ_INIT_NDX(GT, 1,
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Check for a stalled page flip after each vblank
On Tue, Sep 02, 2014 at 02:57:36PM +0100, Chris Wilson wrote: Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth simon.farnswo...@onelan.co.uk Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. I have this plan that we stop using the flip done interrupts, since they're anyway fairly broken on bdw. But I haven't really thought how that would interact with this stall checking. But I guess we can merge this stuff first and figure out the rest when someone gets around to posting a patch for killing flip done. v2: Cleanup, refactor, and rename v3: Only start counting vblanks after the flip command has been seen by the hardware. v4: Record the seqno after we touch the ring, or else there may be no seqno allocated yet. v5: Rebase on mmio-flip. Reported-by: Simon Farnsworth si...@farnz.org.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v4] Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_debugfs.c | 34 +++--- drivers/gpu/drm/i915/i915_irq.c | 84 +++-- drivers/gpu/drm/i915/intel_display.c | 119 --- drivers/gpu/drm/i915/intel_drv.h | 3 + 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 97c257d06729..f7816b4329d6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -518,6 +518,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) { struct drm_info_node *node = m-private; struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; unsigned long flags; struct intel_crtc *crtc; int ret; @@ -537,6 +538,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, No flip due on pipe %c (plane %c)\n, pipe, plane); } else { + u32 addr; + if (atomic_read(work-pending) INTEL_FLIP_COMPLETE) { seq_printf(m, Flip queued on pipe %c (plane %c)\n, pipe, plane); @@ -544,23 +547,34 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, Flip pending (waiting for vsync) on pipe %c (plane %c)\n, pipe, plane); } + if (work-flip_queued_request) { + struct i915_gem_request *rq = work-flip_queued_request; + seq_printf(m, Flip queued on %s at seqno %u, next seqno %u [current breadcrumb %u], completed? %d\n, +rq-engine-name, +rq-seqno, rq-i915-next_seqno, +rq-engine-get_seqno(rq-engine), +__i915_request_complete__wa(rq)); + } else + seq_printf(m, Flip not associated with any ring\n); + seq_printf(m, Flip queued on frame %d, (was ready on frame %d), now %d\n, +work-flip_queued_vblank, +work-flip_ready_vblank, +drm_vblank_count(dev, crtc-pipe));
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Check for a stalled page flip after each vblank
On Tue, Sep 02, 2014 at 05:31:22PM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 02:57:36PM +0100, Chris Wilson wrote: Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth simon.farnswo...@onelan.co.uk Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. I have this plan that we stop using the flip done interrupts, since they're anyway fairly broken on bdw. But I haven't really thought how that would interact with this stall checking. Now that you remind me, I had a plan to rewrite this on top of that future. But I guess we can merge this stuff first and figure out the rest when someone gets around to posting a patch for killing flip done. Yeah, I think this is important in the short term as we have a bug causing system freezes that this at least (+ next patch) allows us to recover from. -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] drm/i915: debug sink dpms aux errors also on enable
On Tue, Sep 02, 2014 at 04:33:52PM +0300, Jani Nikula wrote: Signed-off-by: Jani Nikula jani.nik...@intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/intel_dp.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d7fc2c0e9ba8..f112d7d2f8bd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1553,8 +1553,6 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) if (mode != DRM_MODE_DPMS_ON) { ret = drm_dp_dpcd_writeb(intel_dp-aux, DP_SET_POWER, DP_SET_POWER_D3); - if (ret != 1) - DRM_DEBUG_DRIVER(failed to write sink power state\n); } else { /* * When turning on, we need to retry for 1ms to give the sink @@ -1568,6 +1566,10 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode) msleep(1); } } + + if (ret != 1) + DRM_DEBUG_KMS(failed to %s sink power state\n, + mode == DRM_MODE_DPMS_ON ? enable : disable); } static bool intel_dp_get_hw_state(struct intel_encoder *encoder, -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Check for a stalled page flip after each vblank
On Tue, Sep 02, 2014 at 02:57:36PM +0100, Chris Wilson wrote: Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth simon.farnswo...@onelan.co.uk Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. v2: Cleanup, refactor, and rename v3: Only start counting vblanks after the flip command has been seen by the hardware. v4: Record the seqno after we touch the ring, or else there may be no seqno allocated yet. v5: Rebase on mmio-flip. Reported-by: Simon Farnsworth si...@farnz.org.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v4] Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch This seems to be on top of the s/seqno/request/ patch which moves around flip-enable_stall_check, which this patch here also does. I'm rather confused about the resulting conflict. Especially since it took me a while to understand what happened to enable_stall_check in your request patch, so I don't think I'm qualified to resolve the conflict. Can you please rebase this one here quickly? Thanks, Daniel --- drivers/gpu/drm/i915/i915_debugfs.c | 34 +++--- drivers/gpu/drm/i915/i915_irq.c | 84 +++-- drivers/gpu/drm/i915/intel_display.c | 119 --- drivers/gpu/drm/i915/intel_drv.h | 3 + 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 97c257d06729..f7816b4329d6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -518,6 +518,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) { struct drm_info_node *node = m-private; struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; unsigned long flags; struct intel_crtc *crtc; int ret; @@ -537,6 +538,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, No flip due on pipe %c (plane %c)\n, pipe, plane); } else { + u32 addr; + if (atomic_read(work-pending) INTEL_FLIP_COMPLETE) { seq_printf(m, Flip queued on pipe %c (plane %c)\n, pipe, plane); @@ -544,23 +547,34 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, Flip pending (waiting for vsync) on pipe %c (plane %c)\n, pipe, plane); } + if (work-flip_queued_request) { + struct i915_gem_request *rq = work-flip_queued_request; + seq_printf(m, Flip queued on %s at seqno %u, next seqno %u [current breadcrumb %u], completed? %d\n, +rq-engine-name, +rq-seqno, rq-i915-next_seqno, +rq-engine-get_seqno(rq-engine), +__i915_request_complete__wa(rq)); + } else + seq_printf(m, Flip not associated with any ring\n); + seq_printf(m, Flip queued on frame %d, (was ready on frame %d), now %d\n, +work-flip_queued_vblank, +work-flip_ready_vblank,
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Check for a stalled page flip after each vblank
On Tue, Sep 02, 2014 at 04:52:54PM +0200, Daniel Vetter wrote: On Tue, Sep 02, 2014 at 02:57:36PM +0100, Chris Wilson wrote: Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth simon.farnswo...@onelan.co.uk Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. v2: Cleanup, refactor, and rename v3: Only start counting vblanks after the flip command has been seen by the hardware. v4: Record the seqno after we touch the ring, or else there may be no seqno allocated yet. v5: Rebase on mmio-flip. Reported-by: Simon Farnsworth si...@farnz.org.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v4] Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch This seems to be on top of the s/seqno/request/ patch which moves around flip-enable_stall_check, which this patch here also does. I'm rather confused about the resulting conflict. Especially since it took me a while to understand what happened to enable_stall_check in your request patch, so I don't think I'm qualified to resolve the conflict. Can you please rebase this one here quickly? I have an aversion to seqno/ring combinations. That is so archaic! :) Just these two? I think keeping the requests interface for the boosting is neater than readding more seqno compares. -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] [RFC] drm/i915/edp: use max lanes and clock for edp
On Tue, 02 Sep 2014, Jani Nikula jani.nik...@intel.com wrote: How about throwing this at any eDP link parameter bugs and regressions? Does it feel too much like giving up the battle? Fixes at least one bug... https://bugs.freedesktop.org/show_bug.cgi?id=79386#c15 Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d7fc2c0e9ba8..f4248d7f64f9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -889,23 +889,13 @@ intel_dp_compute_config(struct intel_encoder *encoder, bpp = dev_priv-vbt.edp_bpp; } - if (IS_BROADWELL(dev)) { - /* Yes, it's an ugly hack. */ - min_lane_count = max_lane_count; - DRM_DEBUG_KMS(forcing lane count to max (%u) on BDW\n, - min_lane_count); - } else if (dev_priv-vbt.edp_lanes) { - min_lane_count = min(dev_priv-vbt.edp_lanes, - max_lane_count); - DRM_DEBUG_KMS(using min %u lanes per VBT\n, - min_lane_count); - } - - if (dev_priv-vbt.edp_rate) { - min_clock = min(dev_priv-vbt.edp_rate 3, max_clock); - DRM_DEBUG_KMS(using min %02x link bw per VBT\n, - bws[min_clock]); - } + /* + * Use the maximum clock and number of lanes the eDP panel + * advertizes being capable of. Typically these values + * correspond to the native resolution of the panel. + */ + min_lane_count = max_lane_count; + min_clock = max_clock; } for (; bpp = 6*3; bpp -= 2*3) { -- 1.9.1 -- 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 v2] drm/i915: Fix lock dropping in intel_tv_detect()
Hi! From: ville.syrj...@linux.intel.com Sent: Monday, September 01, 2014 at 12:07 PM When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I tested the posted patch, applying it on top of 3.17-rc2 did solve my boot problems. Just to make sure I also tried a clean 3.17-rc3, which was unable to boot. With this patch applied, it again booted successfully. I'm not quite sure how tags work, but you may add my following tags: Reported-by: Tibor Billes tbil...@gmx.com Tested-by: Tibor Billes tbil...@gmx.com Thank you all for the quick responses and the patch, nice work! :) Tibor ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915/dp: Refactor common eDP lid detection
Both gmch and pch detection routines used the exact same routine for eDP, so de-duplicate. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: : Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d2ee403f72da..0aa134ce114a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3725,20 +3725,24 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) } static enum drm_connector_status +edp_detect(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp_to_dev(intel_dp); + enum drm_connector_status status; + + status = intel_panel_detect(dev); + if (status == connector_status_unknown) + status = connector_status_connected; + + return status; +} + +static enum drm_connector_status ironlake_dp_detect(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev-dev_private; struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); - enum drm_connector_status status; - - /* Can't disconnect eDP, but you can close the lid... */ - if (is_edp(intel_dp)) { - status = intel_panel_detect(dev); - if (status == connector_status_unknown) - status = connector_status_connected; - return status; - } if (!ibx_digital_port_connected(dev_priv, intel_dig_port)) return connector_status_disconnected; @@ -3794,16 +3798,6 @@ g4x_dp_detect(struct intel_dp *intel_dp) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); int ret; - /* Can't disconnect eDP, but you can close the lid... */ - if (is_edp(intel_dp)) { - enum drm_connector_status status; - - status = intel_panel_detect(dev); - if (status == connector_status_unknown) - status = connector_status_connected; - return status; - } - ret = g4x_digital_port_connected(dev, intel_dig_port); if (ret == -EINVAL) return connector_status_unknown; @@ -3877,11 +3871,13 @@ intel_dp_detect(struct drm_connector *connector, bool force) intel_dp-has_audio = false; - if (HAS_PCH_SPLIT(dev)) + /* Can't disconnect eDP, but you can close the lid... */ + if (is_edp(intel_dp)) + status = edp_detect(intel_dp); + else if (HAS_PCH_SPLIT(dev)) status = ironlake_dp_detect(intel_dp); else status = g4x_dp_detect(intel_dp); - if (status != connector_status_connected) goto out; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915/dp: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the EDID found during output discovery and reuse it. This is a separate issue from caching the output EDID across detection cycles. v2: Implement connector-force() callback so that edid is associated with the connector for user overrides as well (Ville) Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 156 ++- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 90 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0aa134ce114a..d5f769788f95 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3808,9 +3808,9 @@ g4x_dp_detect(struct intel_dp *intel_dp) } static struct edid * -intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) +intel_dp_get_edid(struct intel_dp *intel_dp) { - struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_connector *intel_connector = intel_dp-attached_connector; /* use cached edid if we have one */ if (intel_connector-edid) { @@ -3819,27 +3819,55 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) return NULL; return drm_edid_duplicate(intel_connector-edid); - } + } else + return drm_get_edid(intel_connector-base, + intel_dp-aux.ddc); +} - return drm_get_edid(connector, adapter); +static void +intel_dp_set_edid(struct intel_dp *intel_dp) +{ + struct intel_connector *intel_connector = intel_dp-attached_connector; + struct edid *edid; + + edid = intel_dp_get_edid(intel_dp); + intel_connector-detect_edid = edid; + + if (intel_dp-force_audio != HDMI_AUDIO_AUTO) + intel_dp-has_audio = intel_dp-force_audio == HDMI_AUDIO_ON; + else + intel_dp-has_audio = drm_detect_monitor_audio(edid); } -static int -intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter) +static void +intel_dp_unset_edid(struct intel_dp *intel_dp) { - struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_connector *intel_connector = intel_dp-attached_connector; - /* use cached edid if we have one */ - if (intel_connector-edid) { - /* invalid edid */ - if (IS_ERR(intel_connector-edid)) - return 0; + kfree(intel_connector-detect_edid); + intel_connector-detect_edid = NULL; - return intel_connector_update_modes(connector, - intel_connector-edid); - } + intel_dp-has_audio = false; +} - return intel_ddc_get_modes(connector, adapter); +static enum intel_display_power_domain +intel_dp_power_get(struct intel_dp *dp) +{ + struct intel_encoder *encoder = dp_to_dig_port(dp)-base; + enum intel_display_power_domain power_domain; + + power_domain = intel_display_port_power_domain(encoder); + intel_display_power_get(to_i915(encoder-base.dev), power_domain); + + return power_domain; +} + +static void +intel_dp_power_put(struct intel_dp *dp, + enum intel_display_power_domain power_domain) +{ + struct intel_encoder *encoder = dp_to_dig_port(dp)-base; + intel_display_power_put(to_i915(encoder-base.dev), power_domain); } static enum drm_connector_status @@ -3849,27 +3877,22 @@ intel_dp_detect(struct drm_connector *connector, bool force) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct intel_encoder *intel_encoder = intel_dig_port-base; struct drm_device *dev = connector-dev; - struct drm_i915_private *dev_priv = dev-dev_private; enum drm_connector_status status; enum intel_display_power_domain power_domain; - struct edid *edid = NULL; bool ret; - power_domain = intel_display_port_power_domain(intel_encoder); - intel_display_power_get(dev_priv, power_domain); - DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, connector-base.id, connector-name); + intel_dp_unset_edid(intel_dp); if (intel_dp-is_mst) { /* MST devices are disconnected from a monitor POV */ if (intel_encoder-type != INTEL_OUTPUT_EDP) intel_encoder-type = INTEL_OUTPUT_DISPLAYPORT; - status = connector_status_disconnected; - goto out; + return connector_status_disconnected; } - intel_dp-has_audio = false; + power_domain = intel_dp_power_get(intel_dp); /* Can't disconnect
[Intel-gfx] [PATCH 3/5] drm/i915/hdmi: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the EDID found during output discovery and reuse it. This is a separate issue from caching the output EDID across detection cycles. v2: Also hookup the force() callback for audio detection when the user forces the connection status. v3: Ville spots a typo, s/==/!=/ Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 145 +- 1 file changed, 80 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 9169786dbbc3..3b21a769ef54 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -962,104 +962,117 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, return true; } -static enum drm_connector_status -intel_hdmi_detect(struct drm_connector *connector, bool force) +static void +intel_hdmi_unset_edid(struct drm_connector *connector) { - struct drm_device *dev = connector-dev; struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); - struct intel_digital_port *intel_dig_port = - hdmi_to_dig_port(intel_hdmi); - struct intel_encoder *intel_encoder = intel_dig_port-base; - struct drm_i915_private *dev_priv = dev-dev_private; - struct edid *edid; - enum intel_display_power_domain power_domain; - enum drm_connector_status status = connector_status_disconnected; - DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, - connector-base.id, connector-name); + intel_hdmi-has_hdmi_sink = false; + intel_hdmi-has_audio = false; + intel_hdmi-rgb_quant_range_selectable = false; + + kfree(to_intel_connector(connector)-detect_edid); + to_intel_connector(connector)-detect_edid = NULL; +} + +static bool +intel_hdmi_set_edid(struct drm_connector *connector) +{ + struct drm_i915_private *dev_priv = to_i915(connector-dev); + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + struct intel_encoder *intel_encoder = + hdmi_to_dig_port(intel_hdmi)-base; + enum intel_display_power_domain power_domain; + struct edid *edid; + bool connected = false; power_domain = intel_display_port_power_domain(intel_encoder); intel_display_power_get(dev_priv, power_domain); - intel_hdmi-has_hdmi_sink = false; - intel_hdmi-has_audio = false; - intel_hdmi-rgb_quant_range_selectable = false; edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, intel_hdmi-ddc_bus)); - if (edid) { - if (edid-input DRM_EDID_INPUT_DIGITAL) { - status = connector_status_connected; - if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) - intel_hdmi-has_hdmi_sink = - drm_detect_hdmi_monitor(edid); - intel_hdmi-has_audio = drm_detect_monitor_audio(edid); - intel_hdmi-rgb_quant_range_selectable = - drm_rgb_quant_range_selectable(edid); - } - kfree(edid); - } + intel_display_power_put(dev_priv, power_domain); + + to_intel_connector(connector)-detect_edid = edid; + if (edid edid-input DRM_EDID_INPUT_DIGITAL) { + intel_hdmi-rgb_quant_range_selectable = + drm_rgb_quant_range_selectable(edid); - if (status == connector_status_connected) { + intel_hdmi-has_audio = drm_detect_monitor_audio(edid); if (intel_hdmi-force_audio != HDMI_AUDIO_AUTO) intel_hdmi-has_audio = - (intel_hdmi-force_audio == HDMI_AUDIO_ON); - intel_encoder-type = INTEL_OUTPUT_HDMI; + intel_hdmi-force_audio == HDMI_AUDIO_ON; + + if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) + intel_hdmi-has_hdmi_sink = + drm_detect_hdmi_monitor(edid); + + connected = true; } - intel_display_power_put(dev_priv, power_domain); + return connected; +} + +static enum drm_connector_status +intel_hdmi_detect(struct drm_connector *connector, bool force) +{ + enum drm_connector_status status; + + DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, + connector-base.id, connector-name); + + intel_hdmi_unset_edid(connector); + + if (intel_hdmi_set_edid(connector)) { + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + + hdmi_to_dig_port(intel_hdmi)-base.type =
[Intel-gfx] [PATCH 4/5] drm/i915/hdmi: Refactor force_audio - has_audio coupling
The routines for deciding whether we have audio (depending upon force_audio and the associated EDID) are common between detection and set-property. Refactor the code to remove the duplication. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_hdmi.c | 60 ++- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 3b21a769ef54..ad7b523d39a8 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -976,6 +976,30 @@ intel_hdmi_unset_edid(struct drm_connector *connector) } static bool +intel_hdmi_update_audio(struct drm_connector *connector) +{ + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + struct edid *edid = to_intel_connector(connector)-detect_edid; + bool has_audio, has_sink; + bool changed = false; + + if (intel_hdmi-force_audio == HDMI_AUDIO_AUTO) + has_audio = drm_detect_monitor_audio(edid); + else + has_audio = intel_hdmi-force_audio == HDMI_AUDIO_ON; + changed |= intel_hdmi-has_audio |= has_audio; + intel_hdmi-has_audio = has_audio; + + has_sink = false; + if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) + has_sink = drm_detect_hdmi_monitor(edid); + changed |= intel_hdmi-has_hdmi_sink |= has_sink; + intel_hdmi-has_hdmi_sink = has_sink; + + return changed; +} + +static bool intel_hdmi_set_edid(struct drm_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector-dev); @@ -999,16 +1023,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) if (edid edid-input DRM_EDID_INPUT_DIGITAL) { intel_hdmi-rgb_quant_range_selectable = drm_rgb_quant_range_selectable(edid); - - intel_hdmi-has_audio = drm_detect_monitor_audio(edid); - if (intel_hdmi-force_audio != HDMI_AUDIO_AUTO) - intel_hdmi-has_audio = - intel_hdmi-force_audio == HDMI_AUDIO_ON; - - if (intel_hdmi-force_audio != HDMI_AUDIO_OFF_DVI) - intel_hdmi-has_hdmi_sink = - drm_detect_hdmi_monitor(edid); - + intel_hdmi_update_audio(connector); connected = true; } @@ -1064,19 +1079,6 @@ static int intel_hdmi_get_modes(struct drm_connector *connector) return intel_connector_update_modes(connector, edid); } -static bool -intel_hdmi_detect_audio(struct drm_connector *connector) -{ - bool has_audio = false; - struct edid *edid; - - edid = to_intel_connector(connector)-detect_edid; - if (edid edid-input DRM_EDID_INPUT_DIGITAL) - has_audio = drm_detect_monitor_audio(edid); - - return has_audio; -} - static int intel_hdmi_set_property(struct drm_connector *connector, struct drm_property *property, @@ -1094,22 +1096,14 @@ intel_hdmi_set_property(struct drm_connector *connector, if (property == dev_priv-force_audio_property) { enum hdmi_force_audio i = val; - bool has_audio; if (i == intel_hdmi-force_audio) return 0; intel_hdmi-force_audio = i; + if (!intel_hdmi_update_audio(connector)) + return 0; - if (i == HDMI_AUDIO_AUTO) - has_audio = intel_hdmi_detect_audio(connector); - else - has_audio = (i == HDMI_AUDIO_ON); - - if (i == HDMI_AUDIO_OFF_DVI) - intel_hdmi-has_hdmi_sink = 0; - - intel_hdmi-has_audio = has_audio; goto done; } -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915/dp: Refactor force_audio - has_audio coupling
Similar to the previous commit for HDMI, refector the common routine for setting the audio flags between detection and set-property. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_dp.c | 47 ++--- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d5f769788f95..34af2beb34b7 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3824,6 +3824,25 @@ intel_dp_get_edid(struct intel_dp *intel_dp) intel_dp-aux.ddc); } +static bool +intel_dp_update_audio(struct intel_dp *intel_dp) +{ + struct intel_connector *intel_connector = intel_dp-attached_connector; + struct edid *edid = intel_connector-detect_edid ; + bool has_audio; + + if (intel_dp-force_audio != HDMI_AUDIO_AUTO) + has_audio = intel_dp-force_audio == HDMI_AUDIO_ON; + else + has_audio = drm_detect_monitor_audio(edid); + + if (has_audio == intel_dp-has_audio) + return false; + + intel_dp-has_audio = has_audio; + return true; +} + static void intel_dp_set_edid(struct intel_dp *intel_dp) { @@ -3833,10 +3852,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp) edid = intel_dp_get_edid(intel_dp); intel_connector-detect_edid = edid; - if (intel_dp-force_audio != HDMI_AUDIO_AUTO) - intel_dp-has_audio = intel_dp-force_audio == HDMI_AUDIO_ON; - else - intel_dp-has_audio = drm_detect_monitor_audio(edid); + intel_dp_update_audio(intel_dp); } static void @@ -3979,19 +3995,6 @@ static int intel_dp_get_modes(struct drm_connector *connector) return 0; } -static bool -intel_dp_detect_audio(struct drm_connector *connector) -{ - bool has_audio = false; - struct edid *edid; - - edid = to_intel_connector(connector)-detect_edid; - if (edid) - has_audio = drm_detect_monitor_audio(edid); - - return has_audio; -} - static int intel_dp_set_property(struct drm_connector *connector, struct drm_property *property, @@ -4009,22 +4012,14 @@ intel_dp_set_property(struct drm_connector *connector, if (property == dev_priv-force_audio_property) { int i = val; - bool has_audio; if (i == intel_dp-force_audio) return 0; intel_dp-force_audio = i; - - if (i == HDMI_AUDIO_AUTO) - has_audio = intel_dp_detect_audio(connector); - else - has_audio = (i == HDMI_AUDIO_ON); - - if (has_audio == intel_dp-has_audio) + if (!intel_dp_update_audio(intel_dp)) return 0; - intel_dp-has_audio = has_audio; goto done; } -- 2.1.0 ___ 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/hdmi: Refactor force_audio - has_audio coupling
On Tue, Sep 02, 2014 at 08:04:02PM +0100, Chris Wilson wrote: The routines for deciding whether we have audio (depending upon force_audio and the associated EDID) are common between detection and set-property. Refactor the code to remove the duplication. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_hdmi.c | 60 ++- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 3b21a769ef54..ad7b523d39a8 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -976,6 +976,30 @@ intel_hdmi_unset_edid(struct drm_connector *connector) } static bool +intel_hdmi_update_audio(struct drm_connector *connector) +{ + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + struct edid *edid = to_intel_connector(connector)-detect_edid; + bool has_audio, has_sink; + bool changed = false; + + if (intel_hdmi-force_audio == HDMI_AUDIO_AUTO) + has_audio = drm_detect_monitor_audio(edid); + else + has_audio = intel_hdmi-force_audio == HDMI_AUDIO_ON; + changed |= intel_hdmi-has_audio |= has_audio; Oh dear, it is obviously time to give up. -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 -v2 3/4] drm/i915: create struct intel_plane_state
From: Gustavo Padovan gustavo.pado...@collabora.co.uk This new struct will be the storage of src and dst coordinates between the check and commit stages of a plane update. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_drv.h | 20 1 file changed, 20 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4ab0d92..59c1675 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -33,6 +33,7 @@ #include drm/drm_crtc_helper.h #include drm/drm_fb_helper.h #include drm/drm_dp_mst_helper.h +#include drm/drm_rect.h /** * _wait_for - magic (register) wait macro @@ -227,6 +228,25 @@ typedef struct dpll { int p; } intel_clock_t; +struct intel_plane_state { + struct drm_crtc *crtc; + struct drm_framebuffer *fb; + int crtc_x; + int crtc_y; + unsigned int crtc_w; + unsigned int crtc_h; + uint32_t src_x; + uint32_t src_y; + uint32_t src_w; + uint32_t src_h; + struct drm_rect src; + struct drm_rect dst; + struct drm_rect clip; + struct drm_rect orig_src; + struct drm_rect orig_dst; + bool visible; +}; + struct intel_plane_config { bool tiled; int size; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH -v2 4/4] drm/i915: split intel_update_plane into check() and commit()
From: Gustavo Padovan gustavo.pado...@collabora.co.uk Due to the upcoming atomic modesetting feature we need to separate some update functions into a check step that can fail and a commit step that should, ideally, never fail. This commit splits intel_update_plane() and its commit part can still fail due to the fb pinning procedure. v2: use the new struct intel_plane_state to store information between check and commit stages. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_sprite.c | 236 ++-- 1 file changed, 146 insertions(+), 90 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 4cbe286..062eca2 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -845,57 +845,28 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) } static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) +intel_check_sprite_plane(struct drm_plane *plane, +struct intel_plane_state *pstate) { - struct drm_device *dev = plane-dev; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *intel_crtc = to_intel_crtc(pstate-crtc); struct intel_plane *intel_plane = to_intel_plane(plane); - enum pipe pipe = intel_crtc-pipe; + struct drm_framebuffer *fb = pstate-fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb-obj; - struct drm_i915_gem_object *old_obj = intel_plane-obj; - int ret; - bool primary_enabled; - bool visible; + int crtc_x = pstate-crtc_x; + int crtc_y = pstate-crtc_y; + int crtc_w = pstate-crtc_w; + int crtc_h = pstate-crtc_h; + int src_x = pstate-src_x; + int src_y = pstate-src_y; + int src_w = pstate-src_w; + int src_h = pstate-src_h; + struct drm_rect *src = pstate-src; + struct drm_rect *dst = pstate-dst; + struct drm_rect *clip = pstate-clip; int hscale, vscale; int max_scale, min_scale; int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); - struct drm_rect src = { - /* sample coordinates in 16.16 fixed point */ - .x1 = src_x, - .x2 = src_x + src_w, - .y1 = src_y, - .y2 = src_y + src_h, - }; - struct drm_rect dst = { - /* integer pixels */ - .x1 = crtc_x, - .x2 = crtc_x + crtc_w, - .y1 = crtc_y, - .y2 = crtc_y + crtc_h, - }; - const struct drm_rect clip = { - .x2 = intel_crtc-active ? intel_crtc-config.pipe_src_w : 0, - .y2 = intel_crtc-active ? intel_crtc-config.pipe_src_h : 0, - }; - const struct { - int crtc_x, crtc_y; - unsigned int crtc_w, crtc_h; - uint32_t src_x, src_y, src_w, src_h; - } orig = { - .crtc_x = crtc_x, - .crtc_y = crtc_y, - .crtc_w = crtc_w, - .crtc_h = crtc_h, - .src_x = src_x, - .src_y = src_y, - .src_w = src_w, - .src_h = src_h, - }; /* Don't modify another pipe's plane */ if (intel_plane-pipe != intel_crtc-pipe) { @@ -927,55 +898,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, max_scale = intel_plane-max_downscale 16; min_scale = intel_plane-can_scale ? 1 : (1 16); - drm_rect_rotate(src, fb-width 16, fb-height 16, + drm_rect_rotate(src, fb-width 16, fb-height 16, intel_plane-rotation); - hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); BUG_ON(hscale 0); - vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); BUG_ON(vscale 0); - visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale); + pstate-visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale); - crtc_x = dst.x1; - crtc_y = dst.y1; - crtc_w = drm_rect_width(dst); - crtc_h = drm_rect_height(dst); + crtc_x = dst-x1; + crtc_y = dst-y1; + crtc_w = drm_rect_width(dst); + crtc_h = drm_rect_height(dst); - if (visible) { + if (pstate-visible) { /* check again in case clipping clamped the results */ - hscale =
[Intel-gfx] [PATCH -v2 1/4] drm/i915: init sprites with univeral plane init function
From: Derek Foreman derek.fore...@collabora.co.uk Really just for completeness - old init function ends up making the plane exactly the same way due to the way the enums are set up. Signed-off-by: Derek Foreman derek.fore...@collabora.co.uk --- drivers/gpu/drm/i915/intel_sprite.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0bdb00b..4cbe286 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1375,10 +1375,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane-plane = plane; intel_plane-rotation = BIT(DRM_ROTATE_0); possible_crtcs = (1 pipe); - ret = drm_plane_init(dev, intel_plane-base, possible_crtcs, -intel_plane_funcs, -plane_formats, num_plane_formats, -false); + ret = drm_universal_plane_init(dev, intel_plane-base, possible_crtcs, + intel_plane_funcs, + plane_formats, num_plane_formats, + DRM_PLANE_TYPE_OVERLAY); if (ret) { kfree(intel_plane); goto out; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH -v2 2/4] drm/i915: trivial: remove unneed set to NULL
From: Gustavo Padovan gustavo.pado...@collabora.co.uk At this point of the code the obj var is already NULL, so we don't need to set it again to NULL. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/i915/intel_display.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b2e4eac..05937fe 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8253,7 +8253,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, if (!obj) { DRM_DEBUG_KMS(cursor off\n); addr = 0; - obj = NULL; mutex_lock(dev-struct_mutex); goto finish; } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/19] drm: Have the vblank counter account for the time between vblank irq disable and drm_vblank_off()
Hi Ville, went through the vblank rework patch set, mostly looks good to me. I couldn't find any bugs in the code. A first quick test-run on my old Intel GMA-950 (Gen 3'ish i think?) also didn't show apparent problems with the OML_sync_control functions. I'll try to test more carefully with that card and maybe with a few more cards in the next days, if i can get my hands on something more recent. The problematic bits: Patch 3/19 [Don't clear vblank timestamp...] in combination with [5/19 below]: I agree that not clearing the timestamps during drm_vblank_off() is probably the better thing to do for userspace. The idea behind clearing the timestamps was that a ust timestamp of zero signals to userspace that the timestamp is invalid/undefined at the moment, so the client should retry the query if it needs a valid timestamp. This worked in practice insofar as a value of zero can't happen normally, unless a client would query a timestamp during the first microsecond since machine powerup. But i guess returning the last valid (msc, ust) pair to a client during vblank off may be better for things like compositors etc. I also wonder if we ever documented this ust == 0 - -EAGAIN behaviour? The problem with patch 5/19 is gpus/drivers which don't provide precise instantaneous vblank timestamps - which are afaik everything except intel, radeon and nouveau. On such drivers, the old code would return a zero ust timestamp during queries after the first drm_vblank_get() and until the first vblank irq happens and initializes the timestamps to something valid. The zero ust would signal please retry to the client. With patch 5/19 you'd get an updated vblank counter with an outdated vblank timestamp - whatever is stored in the ringbuffer from the past, because drm_update_vblank_count() can't update the timestamp without support for the optional vblank-timestamp driver function. A mismatched msc, ust would be very confusing to clients. The only way one could get valid msc + ust on such drivers would be to enable vblank irq's and then wait for the next vblank irq to actually update the timestamp, at the cost of a couple of msecs waiting time. So either have drm_update_vblank_count() itself sleep until next vblank if (!rc) ... at the very end, as a rc == 0 would signal an imprecise/wrong vblank timestamp. Or have all callers of it do this, if locking makes it neccessary. Or only care about it for the drm_vblank_off() special case, e.g., if !vblank-enabled there, then drm_vblank_get() - wait for a valid timestamp and count to show up - drm_vblank_put() - vblank_disable_and_save(). For Patch 11/19 [Add dev-vblank_disable_immediate flag]: Can we make it so that a drm_vblank_offdelay module parameter of zero always overrides the flag? The only reason why a user wants to set drm_vblank_offdelay to zero is if that user absolutely needs precise and reliable vblank counts/timestamps and finds out that something is broken with his driver+gpu, so uses this as an override to temporarily fix a broken driver. That doesn't work if the vblank_disable_immediate flag overrides the override from the user - the user couldn't opt out of the trouble. This might not be such an issue with Intel cards, as you have test suites and a QA team, and i assume/hope you tested every single intel gpu shipped in the last decade or so if the whole vblank off/on logic really is perfectly race-free now? At least it seems to work with that one gen-3 card i quickly tested. But for most other drivers with small teams and no dedicated QA this could end badly quickly for the user without any manual override. The docs should probably clarify that a hw vblank counter isn't enough for the vblank_disable_immediate flag to be set. Their vblank off/on/hardware counter query implementation must be completely race free. iirc this means the hw counter query must behave as if the vblank counter always increments at the leading edge of vblank. E.g., radeon has hw counter queries, but the counter increments either at the trailing edge, or somewhere in the middle of vblank, so there it wouldn't work without races, causing off-by-one errors sometimes. For Patch 14/19 [Don't update vblank timestamp when the counter didn't change] That would go wrong if a driver doesn't implement a proper vblank counter query. E.g., nouveau has precise vblank timestamping since Linux 3.14, but still no functional hw counter query. Almost all embedded gpu drivers currently implement completely bogus hw vblank counter queries, because that driver hook is mandatory. I think it would make sense if we would make that hook optional, allow a NULL function pointer and adapt to the lack of that query, e.g., by never disabling vblank irq's, except in drm_vblank_off() when a kms-driver insists on disabling its irq during modeset/dpms off/suspend etc. With these remarks somehow taken into account you have my Reviewed-by:
[Intel-gfx] [PATCH] drm/i915: change CHV write_eld/global_resources function pointers
From: Paulo Zanoni paulo.r.zan...@intel.com Currently, CHV is using the same functions as HSW/BDW instead of the same functions as VLV. This looks wrong, especially since, for example, valleyview_modeset_global_resouces even has an IS_CHERRYVIEW check. This patch has the potential to fix display audio and the CHV CDCLK. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) This is completely untested! But maybe it should go to fixes/stable. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c092ff4..e6cae59 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12550,7 +12550,7 @@ static void intel_init_display(struct drm_device *dev) dev_priv-display.write_eld = ironlake_write_eld; dev_priv-display.modeset_global_resources = ivb_modeset_global_resources; - } else if (IS_HASWELL(dev) || IS_GEN8(dev)) { + } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { dev_priv-display.fdi_link_train = hsw_fdi_link_train; dev_priv-display.write_eld = haswell_write_eld; dev_priv-display.modeset_global_resources = -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/7] Rename DP training vswing/pre-emph defines
On Fri, Aug 08, 2014 at 04:23:39PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP 1.4 where the values are different. Updated in all the drivers as well v2: Keeping the old defines in first patch and removing them in last patch. Used cocci semantic patch to replace the defines. Sonika Jindal (7): drm: Renaming DP training vswing pre emph defines drm/i915: Renaming DP training vswing pre emph defines drm/exynos: Renaming DP training vswing pre emph defines drm/gma500: Renaming DP training vswing pre emph defines drm/radeon: Renaming DP training vswing pre emph defines drm/tegra: Renaming DP training vswing pre emph defines drm: Remove old defines for vswing and pre-emph values I've pulled in the entire series with Dave's irc-ack for the non-i915 bits. Thanks, Daniel drivers/gpu/drm/exynos/exynos_dp_core.c |4 +- drivers/gpu/drm/gma500/cdv_intel_dp.c |4 +- drivers/gpu/drm/gma500/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_dp.c | 194 +++ drivers/gpu/drm/radeon/atombios_dp.c|4 +- drivers/gpu/drm/tegra/dpaux.c |4 +- include/drm/drm_dp_helper.h | 16 +-- 8 files changed, 129 insertions(+), 129 deletions(-) -- 1.7.10.4 ___ 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
[Intel-gfx] [PATCH 1/2] drm/i915: Android sync points for i915 v2
Expose an ioctl to create Android fences based on the Android sync point infrastructure (which in turn is based on DMA-buf fences). Just a sketch at this point, no testing has been done. There are a couple of goals here: 1) allow applications and libraries to create fences without an associated buffer 2) re-use a common API so userspace doesn't have to impedance mismatch between different driver implementations too much 3) allow applications and libraries to use explicit synchronization if they choose by exposing fences directly v2: use struct fence directly using Maarten's new interface Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/Kconfig | 2 + drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 10 ++ drivers/gpu/drm/i915/i915_gem.c | 15 +- drivers/gpu/drm/i915/i915_irq.c | 4 +- drivers/gpu/drm/i915/i915_sync.c | 323 +++ include/uapi/drm/i915_drm.h | 23 +++ 8 files changed, 373 insertions(+), 6 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_sync.c diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 4e39ab3..cd0f2ec 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -6,6 +6,8 @@ config DRM_I915 select INTEL_GTT select AGP_INTEL if AGP select INTERVAL_TREE + select ANDROID + select SYNC # we need shmfs for the swappable backing store, and in particular # the shmem_readpage() which depends upon tmpfs select SHMEM diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..61a3eb5c 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_execbuffer.o \ i915_gem_gtt.o \ i915_gem.o \ + i915_sync.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ i915_gem_userptr.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2e7f03a..84086e1 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -2043,6 +2043,7 @@ const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_FENCE, i915_sync_create_fence_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; int i915_max_ioctl = ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d604f4f..6eb119e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1388,6 +1388,8 @@ struct i915_frontbuffer_tracking { unsigned flip_bits; }; +struct i915_sync_timeline; + struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; @@ -1422,6 +1424,8 @@ struct drm_i915_private { struct drm_i915_gem_object *semaphore_obj; uint32_t last_seqno, next_seqno; + struct i915_sync_timeline *sync_tl[I915_NUM_RINGS]; + drm_dma_handle_t *status_page_dmah; struct resource mch_res; @@ -2275,6 +2279,12 @@ void i915_init_vm(struct drm_i915_private *dev_priv, void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); +/* i915_sync.c */ +int i915_sync_init(struct drm_i915_private *dev_priv); +void i915_sync_fini(struct drm_i915_private *dev_priv); +int i915_sync_create_fence_ioctl(struct drm_device *dev, void *data, +struct drm_file *file); + #define PIN_MAPPABLE 0x1 #define PIN_NONBLOCK 0x2 #define PIN_GLOBAL 0x4 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dcd8d7b..ace716e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1146,11 +1146,11 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv) * Returns 0 if the seqno was found within the alloted time. Else returns the * errno with remaining time filled in timeout argument. */ -static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno, - unsigned reset_counter, - bool interruptible, - struct timespec *timeout, - struct drm_i915_file_private *file_priv) +int __wait_seqno(struct intel_engine_cs *ring, u32 seqno, +unsigned reset_counter, +bool interruptible, +struct timespec *timeout, +struct drm_i915_file_private *file_priv) { struct drm_device *dev = ring-dev; struct
[Intel-gfx] Updated fence patches
This set includes a sketch of how we might allow fences to be emitted directly within a batch buffer. This gets rid of the need for flushing around fence operations, which can be a win, and lets userspace more finely control things. If it looks reasonable, we could drop the separate ioctl and just use this instead... Alternately, I could still add the return a fence from execbuf behavior to the main execbuf ioctl, to represent completion for the whole thing. And per the last comments, we still want a way to pass an array of fences in with a given operation, so it can be properly ordered by the scheduler. Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: allow sync points within batches
Use a new reloc type to allow userspace to insert sync points within batches before they're submitted. The corresponding fence fds are returned in the offset field of the returned reloc tree, and can be operated on with the sync fence APIs. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/i915_drv.h| 4 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 125 - drivers/gpu/drm/i915/i915_sync.c | 58 ++--- include/uapi/drm/i915_drm.h| 11 ++- 4 files changed, 167 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6eb119e..410eedf 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2284,6 +2284,10 @@ int i915_sync_init(struct drm_i915_private *dev_priv); void i915_sync_fini(struct drm_i915_private *dev_priv); int i915_sync_create_fence_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +int i915_sync_fence_create(struct intel_engine_cs *ring, + struct intel_context *ctx, + u32 seqno); + #define PIN_MAPPABLE 0x1 #define PIN_NONBLOCK 0x2 diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 60998fc..32ec599 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -32,6 +32,7 @@ #include i915_trace.h #include intel_drv.h #include linux/dma_remapping.h +#include ../../../staging/android/sync.h #define __EXEC_OBJECT_HAS_PIN (131) #define __EXEC_OBJECT_HAS_FENCE (130) @@ -262,6 +263,67 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj) !obj-map_and_fenceable || obj-cache_level != I915_CACHE_NONE); } +static int +emit_sync_obj_cpu(struct drm_i915_gem_object *obj, + struct drm_i915_gem_relocation_entry *reloc) +{ + uint32_t page_offset = offset_in_page(reloc-offset); + char *vaddr; + int ret; + + ret = i915_gem_object_set_to_cpu_domain(obj, true); + if (ret) + return ret; + + vaddr = kmap_atomic(i915_gem_object_get_page(obj, + reloc-offset PAGE_SHIFT)); + *(uint32_t *)(vaddr + page_offset) = MI_STORE_DWORD_INDEX; + *(uint32_t *)(vaddr + page_offset + 4) = + I915_GEM_HWS_INDEX MI_STORE_DWORD_INDEX_SHIFT; + *(uint32_t *)(vaddr + page_offset + 8) = + obj-ring-outstanding_lazy_seqno; + *(uint32_t *)(vaddr + page_offset + 12) = MI_USER_INTERRUPT; + + kunmap_atomic(vaddr); + + return 0; +} + +static int +emit_sync_obj_gtt(struct drm_i915_gem_object *obj, + struct drm_i915_gem_relocation_entry *reloc) +{ + struct drm_device *dev = obj-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + uint32_t __iomem *reloc_entry; + void __iomem *reloc_page; + int ret; + + ret = i915_gem_object_set_to_gtt_domain(obj, true); + if (ret) + return ret; + + ret = i915_gem_object_put_fence(obj); + if (ret) + return ret; + + /* Map the page containing the relocation we're going to perform. */ + reloc-offset += i915_gem_obj_ggtt_offset(obj); + reloc_page = io_mapping_map_atomic_wc(dev_priv-gtt.mappable, + reloc-offset PAGE_MASK); + + reloc_entry = (uint32_t __iomem *) + (reloc_page + offset_in_page(reloc-offset)); + iowrite32(MI_STORE_DWORD_INDEX, reloc_entry); + iowrite32(I915_GEM_HWS_INDEX MI_STORE_DWORD_INDEX_SHIFT, + reloc_entry); + iowrite32(obj-ring-outstanding_lazy_seqno, reloc_entry); + iowrite32(MI_USER_INTERRUPT, reloc_entry); + + io_mapping_unmap_atomic(reloc_page); + + return 0; +} static int relocate_entry_cpu(struct drm_i915_gem_object *obj, @@ -349,7 +411,8 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj, static int i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, struct eb_vmas *eb, - struct drm_i915_gem_relocation_entry *reloc) + struct drm_i915_gem_relocation_entry *reloc, + struct intel_context *ctx) { struct drm_device *dev = obj-base.dev; struct drm_gem_object *target_obj; @@ -433,23 +496,39 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, if (obj-active in_atomic()) return -EFAULT; - if (use_cpu_reloc(obj)) - ret = relocate_entry_cpu(obj, reloc, target_offset); - else - ret = relocate_entry_gtt(obj, reloc, target_offset); + if (reloc-write_domain I915_GEM_DOMAIN_SYNC_OBJ) { + int fd; + +
[Intel-gfx] [PATCH 2/2] drm/i915/hdmi: Enable pipe pixel replication for SD interlaced modes
From: Clint Taylor clinton.a.tay...@intel.com Enable 2x pixel replication for modes the mode flag DBLCLK to double horizontal timings and pixel clock across TMDS. Signed-off-by: Clint Taylor clinton.a.tay...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_hdmi.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 9169786..9695768 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -864,10 +864,15 @@ static enum drm_mode_status intel_hdmi_mode_valid(struct drm_connector *connector, struct drm_display_mode *mode) { - if (mode-clock hdmi_portclock_limit(intel_attached_hdmi(connector), - true)) + int clock = mode-clock; + + if (mode-flags DRM_MODE_FLAG_DBLCLK) + clock *= 2; + + if (clock hdmi_portclock_limit(intel_attached_hdmi(connector), +true)) return MODE_CLOCK_HIGH; - if (mode-clock 2) + if (clock 2) return MODE_CLOCK_LOW; if (mode-flags DRM_MODE_FLAG_DBLSCAN) @@ -921,6 +926,10 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, intel_hdmi-color_range = 0; } + if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK) { + pipe_config-pixel_multiplier = 2; + } + if (intel_hdmi-color_range) pipe_config-limited_color_range = true; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] drm/edid: Reduce horizontal timings for pixel
From: Clint Taylor clinton.a.tay...@intel.com Split original drm_edid.c changes and intel_hdmi.c HDMI pixel replciation changes into separate patches. Clint Taylor (2): drm/edid: Reduce horizontal timings for pixel replicated modes drm/i915/hdmi: Enable pipe pixel replication for SD interlaced modes drivers/gpu/drm/drm_edid.c| 96 ++--- drivers/gpu/drm/i915/intel_hdmi.c | 15 -- 2 files changed, 60 insertions(+), 51 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/edid: Reduce horizontal timings for pixel replicated modes
From: Clint Taylor clinton.a.tay...@intel.com Pixel replicated modes should be non-2x horizontal timings and pixel replicated by the HW across the HDMI cable at 2X pixel clock. Current horizontal resolution of 1440 does not allow pixel duplication to occur and scaling artifacts occur on the TV. HDMI certification 7-26 currently fails for all pixel replicated modes. This change will allow HDMI certification with 480i/576i modes once pixel replication is turned on. Signed-off-by: Clint Taylor clinton.a.tay...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 96 ++-- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index daf3cd8..1bdbfd0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -632,27 +632,27 @@ static const struct drm_display_mode edid_cea_modes[] = { DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_INTERLACE), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 6 - 1440x480i@60Hz */ - { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, - 1602, 1716, 0, 480, 488, 494, 525, 0, + /* 6 - 720(1440)x480i@60Hz */ + { DRM_MODE(720x480i, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, + 801, 858, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 7 - 1440x480i@60Hz */ - { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, - 1602, 1716, 0, 480, 488, 494, 525, 0, + /* 7 - 720(1440)x480i@60Hz */ + { DRM_MODE(720x480i, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, + 801, 858, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 8 - 1440x240@60Hz */ - { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, - 1602, 1716, 0, 240, 244, 247, 262, 0, + /* 8 - 720(1440)x240@60Hz */ + { DRM_MODE(720x240, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, + 801, 858, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 9 - 1440x240@60Hz */ - { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, - 1602, 1716, 0, 240, 244, 247, 262, 0, + /* 9 - 720(1440)x240@60Hz */ + { DRM_MODE(720x240, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, + 801, 858, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, @@ -714,27 +714,27 @@ static const struct drm_display_mode edid_cea_modes[] = { DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_INTERLACE), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 21 - 1440x576i@50Hz */ - { DRM_MODE(1440x576i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, - 1590, 1728, 0, 576, 580, 586, 625, 0, + /* 21 - 720(1440)x576i@50Hz */ + { DRM_MODE(720x576i, DRM_MODE_TYPE_DRIVER, 13500, 720, 732, + 795, 864, 0, 576, 580, 586, 625, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 22 - 1440x576i@50Hz */ - { DRM_MODE(1440x576i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, - 1590, 1728, 0, 576, 580, 586, 625, 0, + /* 22 - 720(1440)x576i@50Hz */ + { DRM_MODE(720x576i, DRM_MODE_TYPE_DRIVER, 13500, 720, 732, + 795, 864, 0, 576, 580, 586, 625, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 23 - 1440x288@50Hz */ - { DRM_MODE(1440x288, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, - 1590, 1728, 0, 288, 290, 293, 312, 0, + /* 23 - 720(1440)x288@50Hz */ + { DRM_MODE(720x288, DRM_MODE_TYPE_DRIVER, 13500, 720, 732, + 795, 864, 0, 288,
Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert
On 08/27/2014 01:54 PM, Runyan, Arthur J wrote: :-) We pulled most of the mobile functions from the bridge chip into the main chip, so that same backlight code might well have been there. From sandybridge onwards I see that hardware will override the PWM signal to inactive (0 if backlight polarity is active high, 1 if active low) when PWM is disabled (PWM control bit 31 = 0). I don't have anything older than sandybridge to look at, and of course there could always be some hidden bug that prevents the override from kicking in. Just enabling/disabling the PWM control bit will reproduce the issue on VLV. I have not tested on other platforms. Clint -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, August 26, 2014 2:52 AM To: Runyan, Arthur J Cc: Jani Nikula; Taylor, Clinton A; Ville Syrjälä; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert On Fri, Aug 22, 2014 at 05:12:25PM +, Runyan, Arthur J wrote: I'll check it out. I haven't heard any complaint about this before, but it may be one of those things that started back on i745 and never got documented. Only i855 and later started to have native lvds with all the surrounding stuff, before there's only bridge chips that did all the magic. So won't be quite _that_ old ;-) Cheers, Daniel -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Friday, August 22, 2014 6:07 AM To: Taylor, Clinton A; Ville Syrjälä; Runyan, Arthur J Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert +Art On Thu, 21 Aug 2014, Clint Taylor clinton.a.tay...@intel.com wrote: There is also a need to add this delay when turning off the PWM enable bit through intel_panel_disable_backlight(). If the PWM enable bit is turned off while the PWM signal is high then the signal remains high. If the bit is turned off when the signal is low the PWM will remain low. Since we don't know the current state of the PWM signal we must set the duty_cycle to 0, wait PWM_CYCLE_DELAY, and then turn off the enable bit. [citation needed] Really, I want this in the specs if this is true. Actually it might be better if we never turn off the PWM enable bit in intel_panel_disable_backlight() and we just use the duty cycle register to control the PWM. Art, any feedback on these two? BR, Jani. So I guess your approach is the simplest option here. _intel_edp_backlight_on(intel_dp); } @@ -4256,9 +4257,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, assign_final(t11_t12); #undef assign_final +#define PWM_CYCLE_DELAY 5 Shoduln't this be calculated from the PWM frequqncy? Not sure what a PWM cycle here is exactly. Just one full period of the signal? The PWM cycle in this case turns out to be 1 full cycle of the PWM waveform though it depends on which display core clock (128 or 25Mhz(S0ix) is being used. Since we only care about the minimum elapsed time to meet the panel specification a value of 5ms will work even though more or less PWM cycles would take place before the BL_ENABLE is asserted. I would prefer not to add complexity to switch between panel timings for normal and S0ix display clock modes. How often does the backlight get enabled while in S0ix? Also would be nice to have a comment in the code explaining what this is and why we need to add it to the delay. Sure, As you may have noticed in other patches I really like comments. #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 ___ 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] [PATCH 2/2] intel/uxa: add MST support.
From: Dave Airlie airl...@redhat.com This adds MST support to the UXA paths in the driver. Signed-off-by: Dave Airlie airl...@redhat.com --- src/uxa/intel.h | 1 + src/uxa/intel_display.c | 345 +++- src/uxa/intel_driver.c | 4 +- 3 files changed, 289 insertions(+), 61 deletions(-) diff --git a/src/uxa/intel.h b/src/uxa/intel.h index 409635d..eebe08f 100644 --- a/src/uxa/intel.h +++ b/src/uxa/intel.h @@ -408,6 +408,7 @@ extern void intel_mode_remove_fb(intel_screen_private *intel); extern void intel_mode_close(intel_screen_private *intel); extern void intel_mode_fini(intel_screen_private *intel); extern int intel_mode_read_drm_events(intel_screen_private *intel); +extern void intel_mode_hotplug(intel_screen_private *intel); typedef void (*intel_drm_handler_proc)(ScrnInfoPtr scrn, xf86CrtcPtr crtc, diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c index 7cdb85f..a3c41f8 100644 --- a/src/uxa/intel_display.c +++ b/src/uxa/intel_display.c @@ -94,6 +94,8 @@ struct intel_mode { void *pageflip_data; intel_pageflip_handler_proc pageflip_handler; intel_pageflip_abort_proc pageflip_abort; + + Bool delete_dp_12_displays; }; struct intel_pageflip { @@ -130,7 +132,7 @@ struct intel_output { struct intel_mode *mode; int output_id; drmModeConnectorPtr mode_output; - drmModeEncoderPtr mode_encoder; + drmModeEncoderPtr *mode_encoders; drmModePropertyBlobPtr edid_blob; int num_props; struct intel_property *props; @@ -145,6 +147,8 @@ struct intel_output { int backlight_active_level; xf86OutputPtr output; struct list link; + int enc_mask; + int enc_clone_mask; }; static void @@ -331,6 +335,9 @@ intel_crtc_apply(xf86CrtcPtr crtc) continue; intel_output = output-driver_private; + if (!intel_output-mode_output) + return FALSE; + output_ids[output_count] = intel_output-mode_output-connector_id; output_count++; @@ -808,6 +815,11 @@ intel_output_attach_edid(xf86OutputPtr output) xf86MonPtr mon = NULL; int i; + if (!koutput) { + xf86OutputSetEDID(output, mon); + return; + } + /* look for an EDID property */ for (i = 0; i koutput-count_props; i++) { drmModePropertyPtr props; @@ -897,6 +909,9 @@ intel_output_get_modes(xf86OutputPtr output) intel_output_attach_edid(output); + if (!koutput) + return Modes; + /* modes should already be available */ for (i = 0; i koutput-count_modes; i++) { DisplayModePtr Mode; @@ -949,7 +964,10 @@ intel_output_destroy(xf86OutputPtr output) free(intel_output-props[i].atoms); } free(intel_output-props); - + for (i = 0; i intel_output-mode_output-count_encoders; i++) { + drmModeFreeEncoder(intel_output-mode_encoders[i]); + } + free(intel_output-mode_encoders); drmModeFreeConnector(intel_output-mode_output); intel_output-mode_output = NULL; @@ -989,6 +1007,9 @@ intel_output_dpms(xf86OutputPtr output, int dpms) struct intel_mode *mode = intel_output-mode; int i; + if (!koutput) + return; + for (i = 0; i koutput-count_props; i++) { drmModePropertyPtr props; @@ -1338,51 +1359,158 @@ static const char *output_names[] = { eDP, }; +static xf86OutputPtr find_output(ScrnInfoPtr pScrn, int id) +{ + xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn); + int i; + for (i = 0; i xf86_config-num_output; i++) { + xf86OutputPtr output = xf86_config-output[i]; + struct intel_output *intel_output; + + intel_output = output-driver_private; + if (intel_output-output_id == id) + return output; + } + return NULL; +} + +static int parse_path_blob(drmModePropertyBlobPtr path_blob, int *conn_base_id, char **path) +{ + char *conn; + char conn_id[5]; + int id, len; + char *blob_data; + + if (!path_blob) + return -1; + + blob_data = path_blob-data; + /* we only handle MST paths for now */ + if (strncmp(blob_data, mst:, 4)) + return -1; + + conn = strchr(blob_data + 4, '-'); + if (!conn) + return -1; + len = conn - (blob_data + 4); + if (len + 1 5) + return -1; + memcpy(conn_id, blob_data + 4, len); + conn_id[len] = '\0'; + id = strtoul(conn_id, NULL, 10); + + *conn_base_id = id; + + *path = conn + 1; + return 0; +} + static void -intel_output_init(ScrnInfoPtr
[Intel-gfx] [PATCH 1/2] intel/uxa/display: drop mode_res caching.
From: Dave Airlie airl...@redhat.com This will cause problems with MST displays. Signed-off-by: Dave Airlie airl...@redhat.com --- src/uxa/intel_display.c | 39 --- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c index 0b83140..7cdb85f 100644 --- a/src/uxa/intel_display.c +++ b/src/uxa/intel_display.c @@ -80,7 +80,6 @@ static struct list intel_drm_queue; struct intel_mode { int fd; uint32_t fb_id; - drmModeResPtr mode_res; int cpp; drmEventContext event_context; @@ -706,7 +705,7 @@ static const xf86CrtcFuncsRec intel_crtc_funcs = { }; static void -intel_crtc_init(ScrnInfoPtr scrn, struct intel_mode *mode, int num) +intel_crtc_init(ScrnInfoPtr scrn, struct intel_mode *mode, drmModeResPtr mode_res, int num) { intel_screen_private *intel = intel_get_screen_private(scrn); xf86CrtcPtr crtc; @@ -723,7 +722,7 @@ intel_crtc_init(ScrnInfoPtr scrn, struct intel_mode *mode, int num) } intel_crtc-mode_crtc = drmModeGetCrtc(mode-fd, - mode-mode_res-crtcs[num]); + mode_res-crtcs[num]); if (intel_crtc-mode_crtc == NULL) { free(intel_crtc); return; @@ -1340,7 +1339,7 @@ static const char *output_names[] = { }; static void -intel_output_init(ScrnInfoPtr scrn, struct intel_mode *mode, int num) +intel_output_init(ScrnInfoPtr scrn, struct intel_mode *mode, drmModeResPtr mode_res, int num) { xf86OutputPtr output; drmModeConnectorPtr koutput; @@ -1350,7 +1349,7 @@ intel_output_init(ScrnInfoPtr scrn, struct intel_mode *mode, int num) char name[32]; koutput = drmModeGetConnector(mode-fd, - mode-mode_res-connectors[num]); + mode_res-connectors[num]); if (!koutput) return; @@ -1381,7 +1380,7 @@ intel_output_init(ScrnInfoPtr scrn, struct intel_mode *mode, int num) return; } - intel_output-output_id = mode-mode_res-connectors[num]; + intel_output-output_id = mode_res-connectors[num]; intel_output-mode_output = koutput; intel_output-mode_encoder = kencoder; intel_output-mode = mode; @@ -1964,10 +1963,10 @@ intel_mode_read_drm_events(struct intel_screen_private *intel) } static drmModeEncoderPtr -intel_get_kencoder(struct intel_mode *mode, int num) +intel_get_kencoder(struct intel_mode *mode, drmModeResPtr mode_res, int num) { struct intel_output *iterator; - int id = mode-mode_res-encoders[num]; + int id = mode_res-encoders[num]; list_for_each_entry(iterator, mode-outputs, link) if (iterator-mode_encoder-encoder_id == id) @@ -1982,7 +1981,7 @@ intel_get_kencoder(struct intel_mode *mode, int num) * values read from libdrm. */ static void -intel_compute_possible_clones(ScrnInfoPtr scrn, struct intel_mode *mode) +intel_compute_possible_clones(ScrnInfoPtr scrn, struct intel_mode *mode, drmModeResPtr mode_res) { xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn); struct intel_output *intel_output, *clone; @@ -2001,7 +2000,7 @@ intel_compute_possible_clones(ScrnInfoPtr scrn, struct intel_mode *mode) if ((mask 1) == 0) continue; - cloned_encoder = intel_get_kencoder(mode, j); + cloned_encoder = intel_get_kencoder(mode, mode_res, j); if (!cloned_encoder) continue; @@ -2024,6 +2023,7 @@ Bool intel_mode_pre_init(ScrnInfoPtr scrn, int fd, int cpp) struct intel_mode *mode; unsigned int i; int has_flipping; + drmModeResPtr mode_res; mode = calloc(1, sizeof *mode); if (!mode) @@ -2037,23 +2037,23 @@ Bool intel_mode_pre_init(ScrnInfoPtr scrn, int fd, int cpp) xf86CrtcConfigInit(scrn, intel_xf86crtc_config_funcs); mode-cpp = cpp; - mode-mode_res = drmModeGetResources(mode-fd); - if (!mode-mode_res) { + mode_res = drmModeGetResources(mode-fd); + if (!mode_res) { xf86DrvMsg(scrn-scrnIndex, X_ERROR, failed to get resources: %s\n, strerror(errno)); free(mode); return FALSE; } - xf86CrtcSetSizeRange(scrn, 320, 200, mode-mode_res-max_width, -mode-mode_res-max_height); - for (i = 0; i mode-mode_res-count_crtcs; i++) - intel_crtc_init(scrn, mode, i); + xf86CrtcSetSizeRange(scrn, 320, 200, mode_res-max_width, +mode_res-max_height); + for (i = 0; i mode_res-count_crtcs; i++) + intel_crtc_init(scrn, mode, mode_res, i); - for
Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert
VLV is a whole different can of worms. Some parts of display are from before sanybridge, and it may have some special muxing on the backlight pins. I won't be able to give any feedback on how hardware behaves there. -Original Message- From: Taylor, Clinton A Sent: Tuesday, September 02, 2014 5:27 PM To: Runyan, Arthur J; Daniel Vetter Cc: Jani Nikula; Ville Syrjälä; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert On 08/27/2014 01:54 PM, Runyan, Arthur J wrote: :-) We pulled most of the mobile functions from the bridge chip into the main chip, so that same backlight code might well have been there. From sandybridge onwards I see that hardware will override the PWM signal to inactive (0 if backlight polarity is active high, 1 if active low) when PWM is disabled (PWM control bit 31 = 0). I don't have anything older than sandybridge to look at, and of course there could always be some hidden bug that prevents the override from kicking in. Just enabling/disabling the PWM control bit will reproduce the issue on VLV. I have not tested on other platforms. Clint -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, August 26, 2014 2:52 AM To: Runyan, Arthur J Cc: Jani Nikula; Taylor, Clinton A; Ville Syrjälä; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert On Fri, Aug 22, 2014 at 05:12:25PM +, Runyan, Arthur J wrote: I'll check it out. I haven't heard any complaint about this before, but it may be one of those things that started back on i745 and never got documented. Only i855 and later started to have native lvds with all the surrounding stuff, before there's only bridge chips that did all the magic. So won't be quite _that_ old ;-) Cheers, Daniel -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Friday, August 22, 2014 6:07 AM To: Taylor, Clinton A; Ville Syrjälä; Runyan, Arthur J Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert +Art On Thu, 21 Aug 2014, Clint Taylor clinton.a.tay...@intel.com wrote: There is also a need to add this delay when turning off the PWM enable bit through intel_panel_disable_backlight(). If the PWM enable bit is turned off while the PWM signal is high then the signal remains high. If the bit is turned off when the signal is low the PWM will remain low. Since we don't know the current state of the PWM signal we must set the duty_cycle to 0, wait PWM_CYCLE_DELAY, and then turn off the enable bit. [citation needed] Really, I want this in the specs if this is true. Actually it might be better if we never turn off the PWM enable bit in intel_panel_disable_backlight() and we just use the duty cycle register to control the PWM. Art, any feedback on these two? BR, Jani. So I guess your approach is the simplest option here. _intel_edp_backlight_on(intel_dp); } @@ -4256,9 +4257,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, assign_final(t11_t12); #undef assign_final +#define PWM_CYCLE_DELAY 5 Shoduln't this be calculated from the PWM frequqncy? Not sure what a PWM cycle here is exactly. Just one full period of the signal? The PWM cycle in this case turns out to be 1 full cycle of the PWM waveform though it depends on which display core clock (128 or 25Mhz(S0ix) is being used. Since we only care about the minimum elapsed time to meet the panel specification a value of 5ms will work even though more or less PWM cycles would take place before the BL_ENABLE is asserted. I would prefer not to add complexity to switch between panel timings for normal and S0ix display clock modes. How often does the backlight get enabled while in S0ix? Also would be nice to have a comment in the code explaining what this is and why we need to add it to the delay. Sure, As you may have noticed in other patches I really like comments. #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