Re: [Intel-gfx] Possible regression in 3.17-rc2 in i915 driver

2014-09-01 Thread Jani Nikula

+intel-gfx

Ville, Daniel, any thoughts before we queue a revert?

BR,
Jani.


On Sun, 31 Aug 2014, Tibor Billes tbil...@gmx.com wrote:
 Hi!

 I tried to upgrade my kernel from 3.16 to 3.17-rc2 and I found that my laptop
 was unable to boot. The boot process hangs after 2-3 seconds (according to
 timestamps of log messages). The last kernel log line I usually see is the
 discovery of my touchpad: input: SynPS/2 Synaptics TouchPad as
 /devices/platform/i8042/serio2/input/input11.

 I did a git bisect and it pointed me to the following commit:
 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()
  
     intel_enable_pipe_a() gets called with all the modeset locks already
     held (by drm_modeset_lock_all()), so trying to grab the same
     locks using another drm_modeset_acquire_ctx is going to fail miserably.
      
     Move most of the drm_modeset_acquire_ctx handling (init/drop/fini)
     out from intel_{get,release}_load_detect_pipe() into the callers
     (intel_{crt,tv}_detect()). Only the actual locking and backoff
     handling is left in intel_get_load_detect_pipe(). And in
     intel_enable_pipe_a() we just share the mode_config.acquire_ctx from
     drm_modeset_lock_all() which is already holding all the relevant locks.
      
     It's perfectly legal to lock the same ww_mutex multiple times using the
     same ww_acquire_ctx. drm_modeset_lock() will convert the returned
     -EALREADY into 0, so the caller doesn't need to do antyhing special.
      
     Fixes a hang on resume on my 830.
      
     Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
     Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
     Cc: sta...@vger.kernel.org
     Signed-off-by: Jani Nikula jani.nik...@intel.com
  
 I tried booting with the above commit reverted on top of 3.17-rc2 and it
 booted successfully.
  
 I also did a MagicSyrq-W (Display list of blocked (D state) tasks) after the
 boot process stopped (using plain 3.17-rc2, without reverting the above 
 commit)
 and it showed that plymouthd was blocked with the following call trace (hand
 copied):
 irq_exit
 common_interrupt
 schedule_preemt_disabled
 __mutex_lock_slowpath
 mutex_lock
 drm_modeset_lock
 drm_getconnector
 drm_mode_getcrttc
 drm_ioctl
 ...
  
 This may have something to do with the hang or it may not, I don't know but is
 related to drm locking so I thought it was a good idea to mention it.
  
 My laptop is an old Fujitsu-Siemens Amilo M1450g, running Linux Mint 16.
  
 Let me know if I can help further debug this issue.
  
 Tibor Billes

-- 
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] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Mike Galbraith
Greetings,

My little Toshiba lappy is exploding non-deterministically during boot
with master (= 3.16 work fine).  I tried to bisect it, but apparently
didn't do quite enough reboots before saying 'good', so ended up...

first bad commit: [5fa9be63a42bd4336448d27cbd1f3b993744dd88]
Merge branch 'msm-fixes-3.17' of git://people.freedesktop.org/~robclark/linux 
into drm-fixes

...touring bisect lala land.  Setting up netconsole, I now have a
picture of the little bugger at least.

[4.881500] [drm] Replacing VGA console driver
[4.889652] checking generic (d000 7ff) vs hw (d000 1000)
[4.897930] fb: switching to inteldrmfb from VESA VGA
[4.906132] Console: switching to colour dummy device 80x25
[4.906138] usb 7-1: new full-speed USB device number 2 using uhci_hcd
[4.928120] i915 :00:02.0: irq 29 for MSI/MSI-X
[4.928156] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[4.928164] [drm] Driver supports precise vblank timestamp query.
[4.928269] vgaarb: device changed decodes: 
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[5.017563] BUG: unable to handle kernel paging request at c900108c4000
[5.017586] IP: [a018704d] gen4_read32+0x3d/0xc0 [i915]
[5.017649] PGD 13f021067 PUD 13f022067 PMD 1361be067 PTE 0
[5.017667] Oops:  [#1] SMP 
[5.017678] Modules linked in: i915(+) drm_kms_helper drm i2c_algo_bit 
processor video thermal thermal_sys button scsi_dh_rdac scsi_dh_alua 
scsi_dh_emc scsi_dh_hp_sw scsi_dh netconsole atl1c
[5.017735] CPU: 0 PID: 54 Comm: kworker/u4:5 Not tainted 3.17.0-master #210
[5.017743] Hardware name: TOSHIBA SATELLITE T130/SATELLITE T130, BIOS V1.70 
   09/29/2009
[5.017788] Workqueue: i915-dp i915_digport_work_func [i915]
[5.017798] task: 8801360de350 ti: 8801360e task.ti: 
8801360e
[5.017805] RIP: 0010:[a018704d]  [a018704d] 
gen4_read32+0x3d/0xc0 [i915]
[5.017857] RSP: 0018:8801360e3d48  EFLAGS: 00010086
[5.017863] RAX: 0297 RBX: 88003729 RCX: 00da
[5.017870] RDX: c900108c4000 RSI: 8800372993c8 RDI: 880037290068
[5.017877] RBP: 8801360e3d70 R08: 0001 R09: a000
[5.017883] R10: dffeab59e7299400 R11: fffedfee R12: 000c4000
[5.017890] R13: 880037290068 R14: 0001 R15: 0001
[5.017897] FS:  () GS:88013fc0() 
knlGS:
[5.017906] CS:  0010 DS:  ES:  CR0: 8005003b
[5.017912] CR2: c900108c4000 CR3: 01a15000 CR4: 000407f0
[5.017918] Stack:
[5.017923]  0080 0011 880037067000 
0001
[5.017939]  880037dcb000 8801360e3d88 a01975c3 
88003729
[5.017955]  8801360e3dc8 a01bd883 880037dcb0e0 
880037299400
[5.017970] Call Trace:
[5.018020]  [a01975c3] ibx_digital_port_connected+0x63/0xb0 [i915]
[5.018071]  [a01bd883] intel_dp_hpd_pulse+0xc3/0x1f0 [i915]
[5.018115]  [a0175b0d] i915_digport_work_func+0x9d/0x120 [i915]
[5.018128]  [81060b26] process_one_work+0x186/0x3f0
[5.018136]  [81060eb1] worker_thread+0x121/0x480
[5.018145]  [81060d90] ? process_one_work+0x3f0/0x3f0
[5.018154]  [81065689] kthread+0xc9/0xe0
[5.018163]  [810655c0] ? kthread_create_on_node+0x170/0x170
[5.018173]  [8157edac] ret_from_fork+0x7c/0xb0
[5.018182]  [810655c0] ? kthread_create_on_node+0x170/0x170
[5.018188] Code: 41 54 49 89 f4 53 48 8d b7 c8 93 00 00 48 89 fb 48 8b 3f 
4c 8d 6b 68 e8 f2 f1 ff ff 4c 89 ef e8 3a 79 3f e1 4c 89 e2 48 03 53 60 44 8b 
32 48 89 c6 4c 89 ef e8 95 75 3f e1 8b 05 c7 0f 08 00 85 
[5.018329] RIP  [a018704d] gen4_read32+0x3d/0xc0 [i915]
[5.018377]  RSP 8801360e3d48
[5.018383] CR2: c900108c4000
[5.018390] ---[ end trace d3d1a2cea94fa73a ]---
[5.018397] Kernel panic - not syncing: Fatal exception
[5.018422] Kernel Offset: 0x0 from 0x8100 (relocation range: 
0x8000-0x9fff)

crash mod -s i915
 MODULE   NAME  SIZE  OBJECT FILE
a01d7760  i915935705  
/lib/modules/3.17.0-master/kernel/drivers/gpu/drm/i915/i915.ko 
crash gdb list *gen4_read32+0x3d
0xa015604d is in gen4_read32 (./arch/x86/include/asm/io.h:56).
51  { asm volatile(mov size  %0,%1: :reg (val), \
52  m (*(volatile type __force *)addr) barrier); }
53  
54  build_mmio_read(readb, b, unsigned char, =q, :memory)
55  build_mmio_read(readw, w, unsigned short, =r, :memory)
56  build_mmio_read(readl, l, unsigned int, =r, :memory)
57  
58  build_mmio_read(__readb, b, unsigned char, =q, )
59  build_mmio_read(__readw, w, unsigned short, =r, )
60  

Re: [Intel-gfx] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Mike Galbraith
Non-deterministic explosion culprit is the below.  The commit says it
adds mysterious acronyms to Haswell, so I suspect my little core2 lappy
isn't ever supposed to see this code.  The ftrace buffer is in fact
empty after a successful boot. 

commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
Author: Dave Airlie airl...@redhat.com
Date:   Fri May 2 14:02:48 2014 +1000

drm/i915: add DP 1.2 MST support (v0.7)

This adds DP 1.2 MST support on Haswell systems.

[4.251017] **
[4.251018] **   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
[4.251019] **  **
[4.251020] ** trace_printk() being used. Allocating extra memory.  **
[4.251021] **  **
[4.251021] ** This means that this is a DEBUG kernel and it is **
[4.251022] ** unsafe for produciton use.   **
[4.251023] **  **
[4.251024] ** If you see this message and you are not debugging**
[4.251024] ** the kernel, report this immediately to your vendor!  **
[4.251025] **  **
[4.251026] **   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
[4.251027] **
[4.375162] [drm] Memory usable by graphics device = 2048M
[4.382963] [drm] Replacing VGA console driver
[4.390710] checking generic (d000 7ff) vs hw (d000 1000)
[4.398617] fb: switching to inteldrmfb from VESA VGA
[4.406578] Console: switching to colour dummy device 80x25
[4.432283] i915 :00:02.0: irq 29 for MSI/MSI-X
[4.432327] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[4.432335] [drm] Driver supports precise vblank timestamp query.
[4.432437] vgaarb: device changed decodes: 
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[4.521746] BUG: unable to handle kernel paging request at c900108c4000
[4.521781] IP: [a014323d] gen4_read32+0x3d/0xc0 [i915]
[4.521843] PGD 13f021067 PUD 13f022067 PMD 136193067 PTE 0
[4.521860] Oops:  [#1] SMP 
[4.521872] Dumping ftrace buffer:
[4.521880] -
[4.521936]udevd-120 1d.h. 4672095us : i9xx_hpd_irq_handler: PIN 1 
continue
[4.521987]udevd-120 1d.h. 4672097us : i9xx_hpd_irq_handler: PIN 2 
continue
[4.522036]udevd-120 1d.h. 4672098us : i9xx_hpd_irq_handler: PIN 3 
continue
[4.522085]udevd-120 1d.h. 4672099us : i9xx_hpd_irq_handler: PIN 4 
continue
[4.522134]udevd-120 1d.h. 4672099us : i9xx_hpd_irq_handler: PIN 5 
continue
[4.522183]udevd-120 1d.h. 4672100us : i9xx_hpd_irq_handler: PORT 3 
PIN 6 continue
[4.522233]udevd-120 1d.h. 4672101us : i9xx_hpd_irq_handler: PORT 3 
PIN 6 sets queue_dig = true
[4.522283]udevd-120 1d.h. 4672102us : i9xx_hpd_irq_handler: QUEUE
[4.522331]...-6   0 4672125us : i915_digport_work_func: WORK
[4.522339] -
[4.522345] Modules linked in: i915(+) drm_kms_helper drm i2c_algo_bit 
thermal video processor thermal_sys button scsi_dh_rdac scsi_dh_alua 
scsi_dh_emc scsi_dh_hp_sw scsi_dh netconsole atl1c
[4.522401] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 3.17.0-bisect #222
[4.522408] Hardware name: TOSHIBA SATELLITE T130/SATELLITE T130, BIOS V1.70 
   09/29/2009
[4.522453] Workqueue: i915-dp i915_digport_work_func [i915]
[4.522462] task: 88013f166150 ti: 88013f168000 task.ti: 
88013f168000
[4.522470] RIP: 0010:[a014323d]  [a014323d] 
gen4_read32+0x3d/0xc0 [i915]
[4.522520] RSP: 0018:88013f16bd48  EFLAGS: 00010086
[4.522527] RAX: 0297 RBX: 88013b1f RCX: 00da
[4.522533] RDX: c900108c4000 RSI: 88013b1f93c8 RDI: 88013b1f0068
[4.522540] RBP: 88013f16bd70 R08: 0001 R09: 0024
[4.522547] R10: 0001167af8eb R11: 0006 R12: 000c4000
[4.522553] R13: 88013b1f0068 R14: 0001 R15: 0001
[4.522560] FS:  () GS:88013fc0() 
knlGS:
[4.522568] CS:  0010 DS:  ES:  CR0: 8005003b
[4.522575] CR2: c900108c4000 CR3: 01a15000 CR4: 000407f0
[4.522581] Stack:
[4.522586]  0080 0011 88013584e000 
0001
[4.522601]  880036c92000 88013f16bd88 a01537b3 
88013b1f
[4.522617]  88013f16bdc8 a0179a73 880036c920e0 
88013b1f9400
[4.522632] Call Trace:
[4.522681]  [a01537b3] ibx_digital_port_connected+0x63/0xb0 [i915]
[4.522733]  [a0179a73] 

Re: [Intel-gfx] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Dave Airlie
 drm/i915: add DP 1.2 MST support (v0.7)

 This adds DP 1.2 MST support on Haswell systems.

I've attached a patch that might fix this, please test and let me know.

Dave.
From d407c946fbf5c48f30160591f5bd71fbe158aeb4 Mon Sep 17 00:00:00 2001
From: Dave Airlie airl...@redhat.com
Date: Mon, 1 Sep 2014 16:58:12 +1000
Subject: [PATCH] drm/i915: handle G45/GM45 pulse detection connected state.

In the HPD pulse handler we check for long pulses if the port is actually
connected, however we do that for IBX, but we use the pulse handling code on
GM45 systems as well, so we need to use a diffent check.

This patch refactors the digital port connected check out of the g4x detection
path and reuses it in the hpd pulse path.

Should fix:
Message-ID: 1409382202.5141.36.ca...@marge.simpson.net

Reported-by: Mike Galbraith umgwanakikb...@gmail.com
Signed-off-by: Dave Airlie airl...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c | 55 +++--
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 67cfed6..81d7681 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3661,24 +3661,12 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 	return intel_dp_detect_dpcd(intel_dp);
 }
 
-static enum drm_connector_status
-g4x_dp_detect(struct intel_dp *intel_dp)
+static int g4x_digital_port_connected(struct drm_device *dev,
+   struct intel_digital_port *intel_dig_port)
 {
-	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);
 	uint32_t bit;
 
-	/* 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;
-	}
-
 	if (IS_VALLEYVIEW(dev)) {
 		switch (intel_dig_port-port) {
 		case PORT_B:
@@ -3691,7 +3679,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 			bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
 			break;
 		default:
-			return connector_status_unknown;
+			return -EINVAL;
 		}
 	} else {
 		switch (intel_dig_port-port) {
@@ -3705,11 +3693,36 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 			bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
 			break;
 		default:
-			return connector_status_unknown;
+			return -EINVAL;
 		}
 	}
 
 	if ((I915_READ(PORT_HOTPLUG_STAT)  bit) == 0)
+		return 0;
+	return 1;
+}
+
+static enum drm_connector_status
+g4x_dp_detect(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(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;
+	else if (ret == 0)
 		return connector_status_disconnected;
 
 	return intel_dp_detect_dpcd(intel_dp);
@@ -4066,8 +4079,14 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	intel_display_power_get(dev_priv, power_domain);
 
 	if (long_hpd) {
-		if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-			goto mst_fail;
+
+		if (HAS_PCH_SPLIT(dev)) {
+			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+goto mst_fail;
+		} else {
+			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
+goto mst_fail;
+		}
 
 		if (!intel_dp_get_dpcd(intel_dp)) {
 			goto mst_fail;
-- 
1.9.3

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


Re: [Intel-gfx] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Mike Galbraith
On Mon, 2014-09-01 at 17:02 +1000, Dave Airlie wrote: 
  drm/i915: add DP 1.2 MST support (v0.7)
 
  This adds DP 1.2 MST support on Haswell systems.
 
 I've attached a patch that might fix this, please test and let me know.

Lappy hasn't exploded in 20 boot cycles, which judging by previous
behavior should mean all is well (knocks anti-deterministic wood).
Thanks.

-Mike

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


[Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread ville . syrjala
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()

Cc: Tibor Billes tbil...@gmx.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_tv.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..abbf0ea 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool 
force)
 {
struct drm_display_mode mode;
struct intel_tv *intel_tv = intel_attached_tv(connector);
-   int type;
+   enum drm_connector_status status = connector-status;
+   int type = -1;
 
DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
  connector-base.id, connector-name,
@@ -1328,21 +1329,23 @@ 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;
+   return status;
 
intel_tv-type = type;
intel_tv_find_better_format(connector);
 
-   return connector_status_connected;
+   return status;
 }
 
 static const struct input_res {
-- 
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 4/7] drm/i915: Improved w/a for rps on Baytrail

2014-09-01 Thread Ville Syrjälä
On Thu, Jul 10, 2014 at 08:31:21PM +0100, Chris Wilson wrote:
 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  | 166 
 ---
  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, 73 insertions(+), 114 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 8e19d031c05d..2db5dbb87ced 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
snip
 @@ -1433,14 +1376,14 @@ static void gen6_pm_rps_work(struct work_struct *work)
  
   mutex_lock(dev_priv-rps.hw_lock);
  
 + pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir);
 +
   adj = dev_priv-rps.last_adj;
   if (pm_iir  GEN6_PM_RP_UP_THRESHOLD) {
   if (adj  0)
   adj *= 2;
 - else {
 - /* CHV needs even encode values */
 - adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
 - }
 + else
 + adj = 1;
   new_delay = dev_priv-rps.cur_freq + adj;
  
   /*
 @@ -1455,15 +1398,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
   else
   new_delay = dev_priv-rps.min_freq_softlimit;
   adj = 0;
 - } else if (pm_iir  GEN6_PM_RP_UP_EI_EXPIRED) {
 - new_delay = vlv_calc_delay_from_C0_counters(dev_priv);
   } 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) ? -2 : -1;
 - }
 + else
 + adj = -1;
   new_delay = dev_priv-rps.cur_freq + adj;
   } else { /* unknown event */
   new_delay = dev_priv-rps.cur_freq;
 @@ -1475,6 +1414,9 @@ static void gen6_pm_rps_work(struct work_struct *work)
   new_delay = clamp_t(int, new_delay,
   dev_priv-rps.min_freq_softlimit,
   dev_priv-rps.max_freq_softlimit);
 + /* CHV needs even encode values */
 + if (IS_CHERRYVIEW(dev_priv))
 + new_delay = new_delay  ~1;

This will effectively make the first up interrupt a nop. The current
code is the way it is precisely to avoid that. I guess it's not a huge
problem but still seems silly to not satisfy the GPU when it wants moar
speed.

-- 
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] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

2014-09-01 Thread Deepak S


On Thursday 28 August 2014 11:00 AM, Daniel Vetter wrote:

On Fri, Aug 29, 2014 at 08:45:21AM +0530, Deepak S wrote:

On Tuesday 26 August 2014 07:24 PM, Daniel Vetter wrote:

On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepa...@linux.intel.com wrote:

From: Deepak S deepa...@linux.intel.com

Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8

This is a regression from
 commit abd58f0175915bed644aa67c8f69dc571b8280e0
 Author: Ben Widawsky benjamin.widaw...@intel.com
 Date:   Sat Nov 2 21:07:09 2013 -0700

drm/i915/bdw: Implement interrupt changes

_Really_ unlikely that this is a regression from this commit, since that
introduced all the gen8 interrupt handling in the first place. I think
this is because of the reworked interrupt handling over gpu resets, where
we want to keep rps interrupts enabled. But I'm not terribly sure.

I think a more convincing story is that this is an oversight from

commit a6706b45a57a23a613b34793e1414991b60a09c1
Author: Deepak S deepa...@linux.intel.com
Date:   Sat Mar 15 20:23:22 2014 +0530

 drm/i915: Track the enabled PM interrupts in dev_priv

or more precisely (since chv wasn't public back then iirc) we forgot to
add the corresponding patch to -internal.

In any case please clarify the commit message and please also add a few
words about why exactly we need this - I had to dig through git history to
figure this all out.

I've applied the patch already, so you can just reply with the revised
commit message that I should put in.

Thanks, Daniel

Daniel, This patch is not just for chv right. it affects the gen8(BDW).
Your right we might have missed in (drm/i915: Track the enabled PM interrupts 
in dev_priv).
In -internal, for chv, We used to program the IER in enable_interrupts routing 
so it was already taken care.
After the interrupt re-work, we are supposed to add IER in post_irq handler 
which we missed it.

New commit msg:  Is this fine? Do i need to resubmit the patch?

Yeah fine and thanks for confirming. I've adjusted the commit message.
-Daniel


Thanks Daniel.


--

Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8

We forgot to program PM IER interrupt in gen8_gt_irq_postinstall based on the 
new  re-worked interrupt
routines.


v2: Kill the loop and init GT interrupts (Ville)

Signed-off-by: Deepak S deepa...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_irq.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5445e7..c33cf89 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device 
*dev)
  static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
  {
-   int i;
-
/* These are interrupts we'll toggle with the ring mask register */
uint32_t gt_interrupts[] = {
GT_RENDER_USER_INTERRUPT  GEN8_RCS_IRQ_SHIFT |
@@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct 
drm_i915_private *dev_priv)
GT_CONTEXT_SWITCH_INTERRUPT  GEN8_VECS_IRQ_SHIFT
};
-   for (i = 0; i  ARRAY_SIZE(gt_interrupts); i++)
-   GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
-
dev_priv-pm_irq_mask = 0x;
+   GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
+   GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
+   GEN8_IRQ_INIT_NDX(GT, 2, dev_priv-pm_irq_mask, 
dev_priv-pm_rps_events);
+   GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
  }
  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
--
1.9.1

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


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


Re: [Intel-gfx] [PATCH 4/7] drm/i915: Improved w/a for rps on Baytrail

2014-09-01 Thread Chris Wilson
On Mon, Sep 01, 2014 at 11:23:20AM +0300, Ville Syrjälä wrote:
 On Thu, Jul 10, 2014 at 08:31:21PM +0100, Chris Wilson wrote:
  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  | 166 
  ---
   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, 73 insertions(+), 114 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_irq.c 
  b/drivers/gpu/drm/i915/i915_irq.c
  index 8e19d031c05d..2db5dbb87ced 100644
  --- a/drivers/gpu/drm/i915/i915_irq.c
  +++ b/drivers/gpu/drm/i915/i915_irq.c
 snip
  @@ -1433,14 +1376,14 @@ static void gen6_pm_rps_work(struct work_struct 
  *work)
   
  mutex_lock(dev_priv-rps.hw_lock);
   
  +   pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir);
  +
  adj = dev_priv-rps.last_adj;
  if (pm_iir  GEN6_PM_RP_UP_THRESHOLD) {
  if (adj  0)
  adj *= 2;
  -   else {
  -   /* CHV needs even encode values */
  -   adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
  -   }
  +   else
  +   adj = 1;
  new_delay = dev_priv-rps.cur_freq + adj;
   
  /*
  @@ -1455,15 +1398,11 @@ static void gen6_pm_rps_work(struct work_struct 
  *work)
  else
  new_delay = dev_priv-rps.min_freq_softlimit;
  adj = 0;
  -   } else if (pm_iir  GEN6_PM_RP_UP_EI_EXPIRED) {
  -   new_delay = vlv_calc_delay_from_C0_counters(dev_priv);
  } 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) ? -2 : -1;
  -   }
  +   else
  +   adj = -1;
  new_delay = dev_priv-rps.cur_freq + adj;
  } else { /* unknown event */
  new_delay = dev_priv-rps.cur_freq;
  @@ -1475,6 +1414,9 @@ static void gen6_pm_rps_work(struct work_struct *work)
  new_delay = clamp_t(int, new_delay,
  dev_priv-rps.min_freq_softlimit,
  dev_priv-rps.max_freq_softlimit);
  +   /* CHV needs even encode values */
  +   if (IS_CHERRYVIEW(dev_priv))
  +   new_delay = new_delay  ~1;
 
 This will effectively make the first up interrupt a nop. The current
 code is the way it is precisely to avoid that. I guess it's not a huge
 problem but still seems silly to not satisfy the GPU when it wants moar
 speed.

Hmm, it's actually worse than that (see the last_adj). Ok, I should
split this out and tidy it up more carefully.
-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/16] drm/i915: Init important ns2501 registers

2014-09-01 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 01:22:03AM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 In my earlier rewrite I missed a few important registers. Thomas Richter
 noticed that they're needed to make his machine resume correctly.
 
 Looks like IEGD does a one time init of these three registers. We don't
 have a good one time init place in the ns2501 driver, so let's just
 stick them into the .mode_set() hook and see if that helps things along.

We have the encoder-reset hooks which are commonly used for such stuff.
Not worth to wire that up for dvo, just an fyi really.
-Daniel

 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/dvo_ns2501.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c 
 b/drivers/gpu/drm/i915/dvo_ns2501.c
 index b278571..345235b 100644
 --- a/drivers/gpu/drm/i915/dvo_ns2501.c
 +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
 @@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
   },
  };
  
 +static const struct ns2501_reg regs_init[] = {
 + [0] = { .offset = 0x35, .value = 0xff, },
 + [1] = { .offset = 0x34, .value = 0x00, },
 + [2] = { .offset = 0x08, .value = 0x30, },
 +};
 +
  struct ns2501_priv {
   bool quiet;
   const struct ns2501_reg *regs;
 @@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
   else
   return;
  
 + /* Hopefully doing it every time won't hurt... */
 + for (i = 0; i  ARRAY_SIZE(regs_init); i++)
 + ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
 +
   ns-regs = regs_1024x768[mode_idx];
  
   for (i = 0; i  84; i++)
 -- 
 1.8.5.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


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

2014-09-01 Thread Daniel Vetter
On Fri, Aug 15, 2014 at 01:22:05AM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 The spec says:
 For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
 GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
 (DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
 both the DPLL A Control Register (06014h-06017h) and DPLL B Control
 Register (06018h-0601Bh).
 
 The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
 just need a bit of special care to handle DPLL_DVO_2X_MODE.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h  |  3 +++
  drivers/gpu/drm/i915/intel_display.c | 47 
 +---
  2 files changed, 46 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 541fb6f..54895a6 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1561,6 +1561,9 @@ struct drm_i915_private {
  
   u16 orig_clock;
  
 + /* used to control DVO 2x clock enable on 830M */
 + uint8_t dvo_pipes;
 +
   bool mchbar_need_disable;
  
   struct intel_l3_parity l3_parity;
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 3eeb5ce..6462bcf 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1548,6 +1548,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
  {
   struct drm_device *dev = crtc-base.dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
 + enum pipe pipe = crtc-pipe;
   int reg = DPLL(crtc-pipe);
   u32 dpll = crtc-config.dpll_hw_state.dpll;
  
 @@ -1560,7 +1561,16 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
   if (IS_MOBILE(dev)  !IS_I830(dev))
   assert_panel_unlocked(dev_priv, crtc-pipe);
  
 - I915_WRITE(reg, dpll);
 + /* enable DVO 2x clock on both PLLs if necessary */
 + if (IS_I830(dev)) {
 + if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
 + dev_priv-dvo_pipes |= 1  pipe;
 +
 + if (dev_priv-dvo_pipes) {
 + dpll |= DPLL_DVO_2X_MODE;
 + I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) | 
 DPLL_DVO_2X_MODE);
 + }
 + }
  
   /* Wait for the clocks to stabilize. */
   POSTING_READ(reg);
 @@ -1599,8 +1609,23 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
   *
   * Note!  This is for pre-ILK only.
   */
 -static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe 
 pipe)
 +static void i9xx_disable_pll(struct intel_crtc *crtc)
  {
 + struct drm_device *dev = crtc-base.dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + enum pipe pipe = crtc-pipe;
 +
 + /* disable DVO 2x clock on both PLLs if necessary */
 + if (IS_I830(dev)) {
 + if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
 + dev_priv-dvo_pipes = ~(1  pipe);
 +
 + if (!dev_priv-dvo_pipes) {
 + I915_WRITE(DPLL(pipe), I915_READ(DPLL(pipe))  
 ~DPLL_DVO_2X_MODE);
 + I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe))  
 ~DPLL_DVO_2X_MODE);

Bikeshed: I'd just use explicit PIPE_A and PIPE_B here, except when we
really have to turn of our pipe first before the other.

 + }
 + }
 +
   /* Don't disable pipe A or pipe A PLLs if needed */
   if (pipe == PIPE_A  (dev_priv-quirks  QUIRK_PIPEA_FORCE))
   return;
 @@ -4788,7 +4813,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
   else if (IS_VALLEYVIEW(dev))
   vlv_disable_pll(dev_priv, pipe);
   else
 - i9xx_disable_pll(dev_priv, pipe);
 + i9xx_disable_pll(intel_crtc);
   }
  
   if (!IS_GEN2(dev))
 @@ -5792,7 +5817,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
   dpll |= PLL_P2_DIVIDE_BY_4;
   }
  
 - if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
 + if (!IS_I830(dev)  intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DVO))
   dpll |= DPLL_DVO_2X_MODE;
  
   if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS) 
 @@ -6298,6 +6323,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc 
 *crtc,
   }
   pipe_config-dpll_hw_state.dpll = I915_READ(DPLL(crtc-pipe));
   if (!IS_VALLEYVIEW(dev)) {
 + if (IS_I830(dev))
 + pipe_config-dpll_hw_state.dpll = ~DPLL_DVO_2X_MODE;
 +
   pipe_config-dpll_hw_state.fp0 = I915_READ(FP0(crtc-pipe));
   pipe_config-dpll_hw_state.fp1 = I915_READ(FP1(crtc-pipe));
   } else {
 @@ -13021,6 +13049,17 @@ void intel_modeset_setup_hw_state(struct drm_device 
 *dev,
   }
   }
  
 + /* update dvo_pipes */
 + if 

[Intel-gfx] [PULL] drm-intel-next

2014-09-01 Thread Daniel Vetter
Hi Dave,

drm-intel-next-2014-08-22:
- basic code for execlist, which is the fancy new cmd submission on gen8. Still
  disabled by default (Ben, Oscar Mateo, Thomas Daniel et al)
- remove the useless usage of console_lock for I915_FBDEV=n (Chris)
- clean up relations between ctx and ppgtt
- clean up ppgtt lifetime handling (Michel Thierry)
- various cursor code improvements from Ville
- execbuffer code cleanups and secure batch fixes (Chris)
- prep work for dev - dev_priv transition (Chris)
- some of the prep patches for the seqno - request object transition (Chris)
- various small improvements all over

Plus a fix from Imre to make sure this pull doesn't break suspend/resume
badly on a bunch of machines on top.

Cheers, Daniel


The following changes since commit 2c0827cffca8ac0c654b888c58a1989a5172f007:

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

are available in the git repository at:

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

for you to fetch changes up to 604effb782a8a4d9a20c8af16bcbf86d742db119:

  drm/i915: fix suspend/resume for GENs w/o runtime PM support (2014-08-26 
13:13:03 +0200)


Ben Widawsky (2):
  drm/i915/bdw: Implement context switching (somewhat)
  drm/i915/bdw: Print context state in debugfs

Chris Wilson (12):
  drm/i915: Only perform set-to-gtt domain for objects bound to the global 
gtt
  drm/i915: Force CPU relocations if not GTT mapped
  drm/i915: Remove fenced_gpu_access and pending_fenced_gpu_access
  drm/i915: Copy PCI device id into the device info block
  drm/i915: Double check ring is idle before declaring the GPU wedged
  drm/i915: Agnostic INTEL_INFO
  drm/i915: Pre-validate the NEED_GTTS flag for execbuffer
  drm/i915: Remove redundant list_empty(eb-vmas) tests in execbuffer
  drm/i915: Simplify relocate_entry_gtt() and make 64-bit safe
  drm/i915: Replace __I915__ with typesafe variant
  drm/i915: Localise the fbdev console lock frobbing
  drm/i915: Print captured bo for all VM in error state

Damien Lespiau (5):
  drm/i915: Fix erroneous conversion to u8
  drm/i915: Fix wrong number of HDMI translation entries
  drm/i915: Make intel_disable_shared_dpll() static
  drm/i915: Remove set but unused 'gt_perf_status'
  drm/i915/bdw: Disable execlists by default

Daniel Vetter (18):
  drm/i915: Fix secure dispatch with full ppgtt
  drm/i915: WARN if module opt sanitization goes out of order
  drm/i915/bdw: Add a context and an engine pointers to the ringbuffer
  drm/i915: Some cleanups for the ppgtt lifetime handling
  drm/i915: Track file_priv, not ctx in the ppgtt structure
  drm/i915: Only refcount ppgtt if it actually is one
  drm/i915: Add proper prefix to obj_to_ggtt
  drm/i915: Allow i915_gem_setup_global_gtt to fail
  drm/i915: Fix up checks for aliasing ppgtt
  drm/i915: Rework ppgtt init to no require an aliasing ppgtt
  drm/i915: Initialize the aliasing ppgtt as part of global gtt
  drm/i915: Only track real ppgtt for a context
  drm/i915: Drop create_vm argument to i915_gem_create_context
  drm/i915: Extract common cleanup into i915_ppgtt_release
  drm/i915: Extract commmon global gtt cleanup code
  drm/i915: Cleanup aliasging ppgtt alongside the global gtt
  drm/i915: Track cursor changes as frontbuffer tracking flushes
  drm/i915: Update DRIVER_DATE to 20140822

Imre Deak (1):
  drm/i915: fix suspend/resume for GENs w/o runtime PM support

Michel Thierry (2):
  drm/i915: vma/ppgtt lifetime rules
  drm/i915/bdw: Two-stage execlist submit process

Oscar Mateo (33):
  drm/i915/bdw: New source and header file for LRs, LRCs and Execlists
  drm/i915/bdw: Macro for LRCs and module option for Execlists
  drm/i915/bdw: Initialization for Logical Ring Contexts
  drm/i915/bdw: Introduce one context backing object per engine
  drm/i915/bdw: A bit more advanced LR context alloc/free
  drm/i915/bdw: Allocate ringbuffers for Logical Ring Contexts
  drm/i915/bdw: Populate LR contexts (somewhat)
  drm/i915/bdw: Deferred creation of user-created LRCs
  drm/i915: Abstract the legacy workload submission mechanism away
  drm/i915/bdw: Skeleton for the new logical rings submission path
  drm/i915/bdw: Generic logical ring init and cleanup
  drm/i915/bdw: GEN-specific logical ring init
  drm/i915/bdw: GEN-specific logical ring set/get seqno
  drm/i915/bdw: New logical ring submission mechanism
  drm/i915/bdw: GEN-specific logical ring emit request
  drm/i915/bdw: GEN-specific logical ring emit flush
  drm/i915/bdw: Ring idle and stop with logical rings
  drm/i915/bdw: Interrupts with logical rings
  drm/i915/bdw: GEN-specific logical ring emit batchbuffer start
  drm/i915/bdw: Workload submission mechanism for Execlists
  

Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Chris Wilson
On Mon, Sep 01, 2014 at 11:09:46AM +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()
 
 Cc: Tibor Billes tbil...@gmx.com
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_tv.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
 index 32186a6..abbf0ea 100644
 --- a/drivers/gpu/drm/i915/intel_tv.c
 +++ b/drivers/gpu/drm/i915/intel_tv.c
 @@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool 
 force)
  {
   struct drm_display_mode mode;
   struct intel_tv *intel_tv = intel_attached_tv(connector);
 - int type;
 + enum drm_connector_status status = connector-status;
 + int type = -1;
  
   DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
 connector-base.id, connector-name,
 @@ -1328,21 +1329,23 @@ 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;
 + return status;
  
   intel_tv-type = type;
   intel_tv_find_better_format(connector);
  
 - return connector_status_connected;
 + return status;

Hmm, this makes the code unclear. if type != -1, status should always be
connected.

I think:

if (status != connector_status_connected)
  return status;

if (WARN_ON(type  0))
   return connector_status_disconnected;

intel_tv-type = type;
find_better_format();
return connector_status_connected;

And leave type unset to encourage gcc.



diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..a109b7b 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 = connector-status;
int type;
 
DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
@@ -1327,17 +1328,20 @@ 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);
+   status = type  0 ?
+   connector_status_disconnected :
+   connector_status_connected;
+
intel_release_load_detect_pipe(connector, tmp);
} 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);

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


Re: [Intel-gfx] [PATCH 00/16] drm/i915: 830M/ns201 fixes again

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

Ok, I've merged all the patches here except the dvo 2x mode thing. And imo
we can also pull in the burst size hack since imo that's what we want -
afaik the hw has some bits to select the burst size between 4, 8, ...
entries, and atm we always set it to 8. And Bspec explicitly says that
setting the fifo to something smaller than burst size will lead to tears.

So the patch with a pimped commit message + bit a better comment is good
to go. Originally I wanted to implement proper burst size selection, but
that ran afoul of the general gen2/3 wm mess.

Aside: We might want to give intel_calculate_wm some i9xx prefix, since
it's very much not generic.

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


Re: [Intel-gfx] [PATCH 0/5] A few fixes on top of the wa_regs patches

2014-09-01 Thread Daniel Vetter
On Sun, Aug 31, 2014 at 08:32:55PM +0100, Siluvery, Arun wrote:
 On 30/08/2014 16:50, Damien Lespiau wrote:
 Hi Arun,
 
 I've compiled a few patches that I think solve some small-ish issues around
 your wa_regs series. Could you please have a look at them and comment/give 
 your
 r-b tag if you judge appropriate?
 
 On top of those patches, I'd love some comments on the issues I raised in the
 other mail and possible follow up patches to address them.
 
http://lists.freedesktop.org/archives/intel-gfx/2014-August/051514.html
 
 Hi Damien,
 
 I really appreciate you taking time to not just give review comments but
 also sending patches to fix those issues.
 
 Chris suggested a way of emitting all LRIs using a simple function and I
 really wanted to rework everything based on that suggestion.
 
 The LRIs are now organized in an array as opposed to sending them
 individually also debugfs patch can make use of it. I have removed the
 temporary array included in driver private structure.
 I think now it looks clean and we can easily add new w/a with minimal
 changes.
 
 Since all of the patches are modified I think it is better to squash them
 with the merged ones rather than updating them with new patches so I have
 folded your patches during rework and will send them after testing, please
 review them and give your comments.

Please don't squash fixup patches when I've merged your patch already -
usually I only drop patches when they're terminally broken, so if you send
me a new version I have to fiddle things to make it all apply. But
squashing in a fixup patch is simpler. And imo also easier to review.

And once we deal in fixup patches it's ok to have a bunch of them imo,
too.
-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] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Daniel Vetter
On Mon, Sep 01, 2014 at 05:02:51PM +1000, Dave Airlie wrote:
  drm/i915: add DP 1.2 MST support (v0.7)
 
  This adds DP 1.2 MST support on Haswell systems.
 
 I've attached a patch that might fix this, please test and let me know.
 
 Dave.

 From d407c946fbf5c48f30160591f5bd71fbe158aeb4 Mon Sep 17 00:00:00 2001
 From: Dave Airlie airl...@redhat.com
 Date: Mon, 1 Sep 2014 16:58:12 +1000
 Subject: [PATCH] drm/i915: handle G45/GM45 pulse detection connected state.
 
 In the HPD pulse handler we check for long pulses if the port is actually
 connected, however we do that for IBX, but we use the pulse handling code on
 GM45 systems as well, so we need to use a diffent check.
 
 This patch refactors the digital port connected check out of the g4x detection
 path and reuses it in the hpd pulse path.
 
 Should fix:
 Message-ID: 1409382202.5141.36.ca...@marge.simpson.net
 
 Reported-by: Mike Galbraith umgwanakikb...@gmail.com
 Signed-off-by: Dave Airlie airl...@redhat.com

Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_dp.c | 55 
 +++--
  1 file changed, 37 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 67cfed6..81d7681 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3661,24 +3661,12 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
   return intel_dp_detect_dpcd(intel_dp);
  }
  
 -static enum drm_connector_status
 -g4x_dp_detect(struct intel_dp *intel_dp)
 +static int g4x_digital_port_connected(struct drm_device *dev,
 +struct intel_digital_port 
 *intel_dig_port)
  {
 - 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);
   uint32_t bit;
  
 - /* 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;
 - }
 -
   if (IS_VALLEYVIEW(dev)) {
   switch (intel_dig_port-port) {
   case PORT_B:
 @@ -3691,7 +3679,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
   bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
   break;
   default:
 - return connector_status_unknown;
 + return -EINVAL;
   }
   } else {
   switch (intel_dig_port-port) {
 @@ -3705,11 +3693,36 @@ g4x_dp_detect(struct intel_dp *intel_dp)
   bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
   break;
   default:
 - return connector_status_unknown;
 + return -EINVAL;
   }
   }
  
   if ((I915_READ(PORT_HOTPLUG_STAT)  bit) == 0)
 + return 0;
 + return 1;
 +}
 +
 +static enum drm_connector_status
 +g4x_dp_detect(struct intel_dp *intel_dp)
 +{
 + struct drm_device *dev = intel_dp_to_dev(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;
 + else if (ret == 0)
   return connector_status_disconnected;
  
   return intel_dp_detect_dpcd(intel_dp);
 @@ -4066,8 +4079,14 @@ intel_dp_hpd_pulse(struct intel_digital_port 
 *intel_dig_port, bool long_hpd)
   intel_display_power_get(dev_priv, power_domain);
  
   if (long_hpd) {
 - if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
 - goto mst_fail;
 +
 + if (HAS_PCH_SPLIT(dev)) {
 + if (!ibx_digital_port_connected(dev_priv, 
 intel_dig_port))
 + goto mst_fail;
 + } else {
 + if (g4x_digital_port_connected(dev, intel_dig_port) != 
 1)
 + goto mst_fail;
 + }
  
   if (!intel_dp_get_dpcd(intel_dp)) {
   goto mst_fail;
 -- 
 1.9.3
 


-- 
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

Re: [Intel-gfx] [PATCH 0/5] A few fixes on top of the wa_regs patches

2014-09-01 Thread Siluvery, Arun

On 01/09/2014 10:08, Daniel Vetter wrote:

On Sun, Aug 31, 2014 at 08:32:55PM +0100, Siluvery, Arun wrote:

On 30/08/2014 16:50, Damien Lespiau wrote:

Hi Arun,

I've compiled a few patches that I think solve some small-ish issues around
your wa_regs series. Could you please have a look at them and comment/give your
r-b tag if you judge appropriate?

On top of those patches, I'd love some comments on the issues I raised in the
other mail and possible follow up patches to address them.

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


Hi Damien,

I really appreciate you taking time to not just give review comments but
also sending patches to fix those issues.

Chris suggested a way of emitting all LRIs using a simple function and I
really wanted to rework everything based on that suggestion.

The LRIs are now organized in an array as opposed to sending them
individually also debugfs patch can make use of it. I have removed the
temporary array included in driver private structure.
I think now it looks clean and we can easily add new w/a with minimal
changes.

Since all of the patches are modified I think it is better to squash them
with the merged ones rather than updating them with new patches so I have
folded your patches during rework and will send them after testing, please
review them and give your comments.


Please don't squash fixup patches when I've merged your patch already -
usually I only drop patches when they're terminally broken, so if you send
me a new version I have to fiddle things to make it all apply. But
squashing in a fixup patch is simpler. And imo also easier to review.


In this case all of the code is newly added so most of it should apply 
cleanly but if the preference is to not squash them I can send fix-up 
patches accordingly.


regards
Arun



And once we deal in fixup patches it's ok to have a bunch of them imo,
too.
-Daniel



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


Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Ville Syrjälä
On Mon, Sep 01, 2014 at 09:50:22AM +0100, Chris Wilson wrote:
 On Mon, Sep 01, 2014 at 11:09:46AM +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()
  
  Cc: Tibor Billes tbil...@gmx.com
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/intel_tv.c | 15 +--
   1 file changed, 9 insertions(+), 6 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_tv.c 
  b/drivers/gpu/drm/i915/intel_tv.c
  index 32186a6..abbf0ea 100644
  --- a/drivers/gpu/drm/i915/intel_tv.c
  +++ b/drivers/gpu/drm/i915/intel_tv.c
  @@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool 
  force)
   {
  struct drm_display_mode mode;
  struct intel_tv *intel_tv = intel_attached_tv(connector);
  -   int type;
  +   enum drm_connector_status status = connector-status;
  +   int type = -1;
   
  DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
connector-base.id, connector-name,
  @@ -1328,21 +1329,23 @@ 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;
  +   return status;
   
  intel_tv-type = type;
  intel_tv_find_better_format(connector);
   
  -   return connector_status_connected;
  +   return status;
 
 Hmm, this makes the code unclear. if type != -1, status should always be
 connected.
 
 I think:
 
 if (status != connector_status_connected)
   return status;
 
 if (WARN_ON(type  0))
return connector_status_disconnected;
 
 intel_tv-type = type;
 find_better_format();
 return connector_status_connected;
 
 And leave type unset to encourage gcc.
 
 
 
 diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
 index 32186a6..a109b7b 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 = connector-status;
 int type;
  
 DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
 @@ -1327,17 +1328,20 @@ 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);
 +   status = type  0 ?
 +   connector_status_disconnected :
 +   connector_status_connected;
 +
 intel_release_load_detect_pipe(connector, tmp);
 } 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);

With this approach we're going to have to keep the !force else branch
to avoid populating intel_tv-type with stack garbage. But I suppose
the resulting code might still be a bit clearer.

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Chris Wilson
On Mon, Sep 01, 2014 at 12:45:56PM +0300, Ville Syrjälä wrote:
 On Mon, Sep 01, 2014 at 09:50:22AM +0100, Chris Wilson wrote:
  diff --git a/drivers/gpu/drm/i915/intel_tv.c 
  b/drivers/gpu/drm/i915/intel_tv.c
  index 32186a6..a109b7b 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 = connector-status;
  int type;
   
  DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
  @@ -1327,17 +1328,20 @@ 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);
  +   status = type  0 ?
  +   connector_status_disconnected :
  +   connector_status_connected;
  +
  intel_release_load_detect_pipe(connector, tmp);
  } 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);
 
 With this approach we're going to have to keep the !force else branch
 to avoid populating intel_tv-type with stack garbage. But I suppose
 the resulting code might still be a bit clearer.

Or just set intel_tv-type directly inside

status = intel_tv_detect_type() ?

Hmm. Indeed, we should not be touching them at all for !forced, as type
is only set for forced, and so we can similarly ignore hpd of better
formats.

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a656816..e80eac5e5239 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1169,7 +1169,7 @@ static const struct drm_display_mode reported_modes[] = {
  * \return true if TV is connected.
  * \return false if TV is disconnected.
  */
-static int
+static enum drm_connector_status
 intel_tv_detect_type(struct intel_tv *intel_tv,
  struct drm_connector *connector)
 {
@@ -1178,10 +1178,10 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = encoder-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   enum drm_connector_status status;
unsigned long irqflags;
u32 tv_ctl, save_tv_ctl;
u32 tv_dac, save_tv_dac;
-   int type;
 
/* Disable TV interrupts around load detect or we'll recurse */
if (connector-polled  DRM_CONNECTOR_POLL_HPD) {
@@ -1229,7 +1229,6 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
intel_wait_for_vblank(intel_tv-base.base.dev,
  to_intel_crtc(intel_tv-base.base.crtc)-pipe);
 
-   type = -1;
tv_dac = I915_READ(TV_DAC);
DRM_DEBUG_KMS(TV detected: %x, %x\n, tv_ctl, tv_dac);
/*
@@ -1238,18 +1237,19 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 *  1 0 X svideo
 *  0 0 0 Component
 */
+   status = connector_status_connected;
if ((tv_dac  TVDAC_SENSE_MASK) == (TVDAC_B_SENSE | TVDAC_C_SENSE)) {
DRM_DEBUG_KMS(Detected Composite TV connection\n);
-   type = DRM_MODE_CONNECTOR_Composite;
+   intel_tv-type = DRM_MODE_CONNECTOR_Composite;
} else if ((tv_dac  (TVDAC_A_SENSE|TVDAC_B_SENSE)) == TVDAC_A_SENSE) {
DRM_DEBUG_KMS(Detected S-Video TV connection\n);
-   type = DRM_MODE_CONNECTOR_SVIDEO;
+   intel_tv-type = DRM_MODE_CONNECTOR_SVIDEO;
} else if ((tv_dac  TVDAC_SENSE_MASK) == 0) {
DRM_DEBUG_KMS(Detected Component TV connection\n);
-   type = DRM_MODE_CONNECTOR_Component;
+   intel_tv-type = DRM_MODE_CONNECTOR_Component;
} else {
DRM_DEBUG_KMS(Unrecognised TV connection\n);
-   type = -1;
+   status = connector_status_disconnected;
}
 
I915_WRITE(TV_DAC, save_tv_dac  ~TVDAC_STATE_CHG_EN);
@@ -1269,7 +1269,7 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
}
 
-   return type;
+   return status;
 }
 
 /*
@@ -1309,39 +1309,33 @@ static void intel_tv_find_better_format(struct 
drm_connector *connector)
 

[Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread ville . syrjala
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
---
 drivers/gpu/drm/i915/intel_tv.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..c6b00f20 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,15 +1329,21 @@ 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)
+   if (status != connector_status_connected)
+   return status;
+
+   if (WARN_ON(type  0))
return connector_status_disconnected;
 
intel_tv-type = type;
-- 
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] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Chris Wilson
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.

Otherwise,
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chrsi

-- 
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: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Ville Syrjälä
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.

 
 Otherwise,
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
 -Chrsi
 
 -- 
 Chris Wilson, Intel Open Source Technology Centre

-- 
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] drm/i915: Fix lock dropping in intel_tv_detect()

2014-09-01 Thread Chris Wilson
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. :)
-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 1/5] drm/i915: Rename intel_wa_registers with a i915_ prefix

2014-09-01 Thread Siluvery, Arun

On 30/08/2014 16:50, Damien Lespiau wrote:

Those debugfs files are prefixed by i915, the name of the kernel module,
presumably to make the difference with files exposed by core DRM.

Also, add a ',' at the end of the last entry. This is to ease the
conflict resolution when rebasing internal patches that add a member at
the end of the array. Without it, wiggle can't do its job as we need to
modify an existing line (appending the ',').

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
  drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1467cc1..fc3d582a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2628,7 +2628,7 @@ static int i915_shared_dplls_info(struct seq_file *m, 
void *unused)
return 0;
  }

-static int intel_wa_registers(struct seq_file *m, void *unused)
+static int i915_wa_registers(struct seq_file *m, void *unused)
  {
int i;
int ret;
@@ -4198,7 +4198,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{i915_semaphore_status, i915_semaphore_status, 0},
{i915_shared_dplls_info, i915_shared_dplls_info, 0},
{i915_dp_mst_info, i915_dp_mst_info, 0},
-   {intel_wa_registers, intel_wa_registers, 0}
+   {i915_wa_registers, i915_wa_registers, 0},
  };
  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)



Reviewed-by: Arun Siluvery arun.siluv...@linux.intel.com
This is only for this patch, remaining patches are not required in the 
rework.


regards
Arun

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


[Intel-gfx] [PATCH] drm: i915: reduce memory footprint when debugging

2014-09-01 Thread Andy Shevchenko
There is no need to use hex_dump_to_buffer() since we have a kernel helper to
dump up to 64 bytes just via printk(). In our case the actual size is 15 bytes.

Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6db84bf..00b79f5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3318,15 +3318,11 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
struct drm_device *dev = dig_port-base.base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   char dpcd_hex_dump[sizeof(intel_dp-dpcd) * 3];
-
if (intel_dp_dpcd_read_wake(intel_dp-aux, 0x000, intel_dp-dpcd,
sizeof(intel_dp-dpcd))  0)
return false; /* aux transfer failed */
 
-   hex_dump_to_buffer(intel_dp-dpcd, sizeof(intel_dp-dpcd),
-  32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
-   DRM_DEBUG_KMS(DPCD: %s\n, dpcd_hex_dump);
+   DRM_DEBUG_KMS(DPCD: %*ph\n, sizeof(intel_dp-dpcd), intel_dp-dpcd);
 
if (intel_dp-dpcd[DP_DPCD_REV] == 0)
return false; /* DPCD not present */
-- 
2.1.0

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


Re: [Intel-gfx] [PATCH 10/14] drm/i915: Track which port is using which pipe's power sequencer

2014-09-01 Thread Antti Koskipää
Reviewed-by: Antti Koskipaa antti.koski...@linux.intel.com

-- 
- Antti

On 08/18/2014 10:16 PM, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 VLV/CHV have a per-pipe panel power sequencer which locks onto the
 port once used. We need to keep track wich power sequencers are
 locked to which ports.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c  | 185 
 ++-
  drivers/gpu/drm/i915/intel_drv.h |   6 ++
  2 files changed, 168 insertions(+), 23 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 556e4de..4614e6e 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -294,28 +294,99 @@ static enum pipe
  vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
  {
   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 - struct drm_crtc *crtc = intel_dig_port-base.base.crtc;
   struct drm_device *dev = intel_dig_port-base.base.dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
 - enum port port = intel_dig_port-port;
 - enum pipe pipe;
 + struct intel_encoder *encoder;
 + unsigned int pipes = (1  PIPE_A) | (1  PIPE_B);
 + struct edp_power_seq power_seq;
  
   lockdep_assert_held(dev_priv-pps_mutex);
  
 - /* modeset should have pipe */
 - if (crtc)
 - return to_intel_crtc(crtc)-pipe;
 + if (intel_dp-pps_pipe != INVALID_PIPE)
 + return intel_dp-pps_pipe;
 +
 + /*
 +  * We don't have power sequencer currently.
 +  * Pick one that's not used by other ports.
 +  */
 + list_for_each_entry(encoder, dev-mode_config.encoder_list, base.head) 
 {
 + struct intel_dp *tmp;
 +
 + if (encoder-type != INTEL_OUTPUT_EDP)
 + continue;
 +
 + tmp = enc_to_intel_dp(encoder-base);
 +
 + if (tmp-pps_pipe != INVALID_PIPE)
 + pipes = ~(1  tmp-pps_pipe);
 + }
 +
 + /*
 +  * Didn't find one. This should not happen since there
 +  * are two power sequencers and up to two eDP ports.
 +  */
 + if (WARN_ON(pipes == 0))
 + return PIPE_A;
 +
 + intel_dp-pps_pipe = ffs(pipes) - 1;
 +
 + DRM_DEBUG_KMS(picked pipe %c power sequencer for port %c\n,
 +   pipe_name(intel_dp-pps_pipe), 
 port_name(intel_dig_port-port));
 +
 + /* init power sequencer on this pipe and port */
 + intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
 + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
 +   power_seq);
 +
 + return intel_dp-pps_pipe;
 +}
 +
 +static enum pipe
 +vlv_initial_power_sequencer_pipe(struct drm_i915_private *dev_priv,
 +  enum port port)
 +{
 + enum pipe pipe;
  
 - /* init time, try to find a pipe with this port selected */
   for (pipe = PIPE_A; pipe = PIPE_B; pipe++) {
   u32 port_sel = I915_READ(VLV_PIPE_PP_ON_DELAYS(pipe)) 
   PANEL_PORT_SELECT_MASK;
 - if (port_sel == PANEL_PORT_SELECT_VLV(port))
 - return pipe;
 +
 + if (port_sel != PANEL_PORT_SELECT_VLV(port))
 + continue;
 +
 + return pipe;
 + }
 +
 + return INVALID_PIPE;
 +}
 +
 +static void
 +vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
 +{
 + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 + struct drm_device *dev = intel_dig_port-base.base.dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct edp_power_seq power_seq;
 + enum port port = intel_dig_port-port;
 +
 + 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);
 +
 + /* didn't find one? just let vlv_power_sequencer_pipe() pick one when 
 needed */
 + if (intel_dp-pps_pipe == INVALID_PIPE) {
 + DRM_DEBUG_KMS(no initial power sequencer for port %c\n,
 +   port_name(port));
 + return;
   }
  
 - /* shrug */
 - return PIPE_A;
 + DRM_DEBUG_KMS(initial power sequencer for port %c: pipe %c\n,
 +   port_name(port), pipe_name(intel_dp-pps_pipe));
 +
 + intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
 + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
 +   power_seq);
  }
  
  static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 @@ -2209,6 +2280,76 @@ static void g4x_pre_enable_dp(struct intel_encoder 
 *encoder)
   }
  }
  
 +static void vlv_steal_power_sequencer(struct drm_device *dev,
 +   enum pipe pipe)
 +{
 + 

[Intel-gfx] DRI3 only DDX driver

2014-09-01 Thread Steven Newbury
I tried building the xorg intel ddx driver with only DRI3 support, 
with DRI1 and DRI2 disabled.

glxinfo says direct rendering is enabled, but gives no core contexts*. 

gnome-shell appears to be using software fallback, generally there 
seems to be no hardware acceleration.  I'm wondering if DRI3 isn't 
ever initialising successfully, and now there's no DRI2 to fall back 
to.

What is the minimal kernel version for DRI3?  I'm currently stuck on 
the 3.14 stable series due to an unresolved kernel bug (which I need 
to bisect...), is it too old?

I remember before the intel (SNA) support landed LIBGL_DEBUG=verbose 
used to mention DRI3 failing, I see nothing mentioning DRI3 at all 
now.  Is there some way of determining which DRI extension is in use?  
DRI3 is properly listed in xdpyinfo.


* I also get no core contexts also with xwayland/glx, related?


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


Re: [Intel-gfx] [PATCH 5/5] drm/i915: Don't restrict i915_wa_registers to BDW

2014-09-01 Thread Siluvery, Arun

On 30/08/2014 16:51, Damien Lespiau wrote:

We have CHV code that already makes the test obsolete. Besides, when
num_wa_regs is 0 (platforms not gathering that W/A data), we expose
something sensible already.

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
  drivers/gpu/drm/i915/i915_debugfs.c | 5 -
  1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index fc3d582a..cd4f045 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2636,11 +2636,6 @@ static int i915_wa_registers(struct seq_file *m, void 
*unused)
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;

-   if (!IS_BROADWELL(dev)) {
-   DRM_DEBUG_DRIVER(Workaround table not available !!\n);
-   return -EINVAL;
-   }
-
ret = mutex_lock_interruptible(dev-struct_mutex);
if (ret)
return ret;


This can also be taken, so patches 1 and 5 in this series.
Reviewed-by: Arun Siluvery arun.siluv...@linux.intel.com

regards
Arun

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915/dp: Cache EDID for a detection cycle

2014-09-01 Thread Ville Syrjälä
On Tue, Aug 12, 2014 at 09:36:03AM +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.

My only real concern here is what happens when someone forces the
connector to connected. Given you only populate detect_edid in
-detect() we wouldn't be able to get the modes from the EDID in
that case.

I guess one solution would be to implement the -force() hook and
attempt the edid read there. Looking at the code we should always do
a -detect() or -force() before -get_modes().

 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/intel_dp.c  | 95 
 
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  2 files changed, 29 insertions(+), 67 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index ed37407..1eef55c 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3764,27 +3764,10 @@ intel_dp_get_edid(struct drm_connector *connector, 
 struct i2c_adapter *adapter)
   return drm_get_edid(connector, adapter);
  }
  
 -static int
 -intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter 
 *adapter)
 -{
 - struct intel_connector *intel_connector = to_intel_connector(connector);
 -
 - /* use cached edid if we have one */
 - if (intel_connector-edid) {
 - /* invalid edid */
 - if (IS_ERR(intel_connector-edid))
 - return 0;
 -
 - return intel_connector_update_modes(connector,
 - intel_connector-edid);
 - }
 -
 - return intel_ddc_get_modes(connector, adapter);
 -}
 -
  static enum drm_connector_status
  intel_dp_detect(struct drm_connector *connector, bool force)
  {
 + struct intel_connector *intel_connector = to_intel_connector(connector);
   struct intel_dp *intel_dp = intel_attached_dp(connector);
   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
   struct intel_encoder *intel_encoder = intel_dig_port-base;
 @@ -3795,21 +3778,20 @@ intel_dp_detect(struct drm_connector *connector, bool 
 force)
   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);
 + kfree(intel_connector-detect_edid);
 + intel_connector-detect_edid = NULL;
  
   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_display_port_power_domain(intel_encoder);
 + intel_display_power_get(dev_priv, power_domain);
  
   if (HAS_PCH_SPLIT(dev))
   status = ironlake_dp_detect(intel_dp);
 @@ -3831,15 +3813,13 @@ intel_dp_detect(struct drm_connector *connector, bool 
 force)
   goto out;
   }
  
 - if (intel_dp-force_audio != HDMI_AUDIO_AUTO) {
 + edid = intel_dp_get_edid(connector, intel_dp-aux.ddc);
 + 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 {
 - edid = intel_dp_get_edid(connector, intel_dp-aux.ddc);
 - if (edid) {
 - intel_dp-has_audio = drm_detect_monitor_audio(edid);
 - kfree(edid);
 - }
 - }
 + else
 + intel_dp-has_audio = drm_detect_monitor_audio(edid);
  
   if (intel_encoder-type != INTEL_OUTPUT_EDP)
   intel_encoder-type = INTEL_OUTPUT_DISPLAYPORT;
 @@ -3852,61 +3832,41 @@ out:
  
  static int intel_dp_get_modes(struct drm_connector *connector)
  {
 - struct intel_dp *intel_dp = intel_attached_dp(connector);
 - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 - struct intel_encoder *intel_encoder = intel_dig_port-base;
   struct intel_connector *intel_connector = to_intel_connector(connector);
 - struct drm_device *dev = connector-dev;
 - struct drm_i915_private *dev_priv = dev-dev_private;
 - enum intel_display_power_domain power_domain;
 - int ret;
 -
 - /* We should parse the EDID data and find out if it has an audio sink
 -  */
 -
 - power_domain = intel_display_port_power_domain(intel_encoder);
 - intel_display_power_get(dev_priv, power_domain);
 + struct edid *edid;
  
 - ret 

Re: [Intel-gfx] [PATCH 1/2] drm/i915/dp: Cache EDID for a detection cycle

2014-09-01 Thread Chris Wilson
On Mon, Sep 01, 2014 at 03:05:26PM +0300, Ville Syrjälä wrote:
 On Tue, Aug 12, 2014 at 09:36:03AM +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.
 
 My only real concern here is what happens when someone forces the
 connector to connected. Given you only populate detect_edid in
 -detect() we wouldn't be able to get the modes from the EDID in
 that case.
 
 I guess one solution would be to implement the -force() hook and
 attempt the edid read there. Looking at the code we should always do
 a -detect() or -force() before -get_modes().

That should fix our audio detection in those cases as well. Sounds good.
-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: reduce memory footprint when debugging

2014-09-01 Thread Jani Nikula
On Mon, 01 Sep 2014, Andy Shevchenko andriy.shevche...@linux.intel.com wrote:
 There is no need to use hex_dump_to_buffer() since we have a kernel helper to
 dump up to 64 bytes just via printk(). In our case the actual size is 15 
 bytes.

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

 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 6db84bf..00b79f5 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3318,15 +3318,11 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
   struct drm_device *dev = dig_port-base.base.dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
  
 - char dpcd_hex_dump[sizeof(intel_dp-dpcd) * 3];
 -
   if (intel_dp_dpcd_read_wake(intel_dp-aux, 0x000, intel_dp-dpcd,
   sizeof(intel_dp-dpcd))  0)
   return false; /* aux transfer failed */
  
 - hex_dump_to_buffer(intel_dp-dpcd, sizeof(intel_dp-dpcd),
 -32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
 - DRM_DEBUG_KMS(DPCD: %s\n, dpcd_hex_dump);
 + DRM_DEBUG_KMS(DPCD: %*ph\n, sizeof(intel_dp-dpcd), intel_dp-dpcd);
  
   if (intel_dp-dpcd[DP_DPCD_REV] == 0)
   return false; /* DPCD not present */
 -- 
 2.1.0

 ___
 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/edid: Reduce horizontal timings for pixel replicated modes

2014-09-01 Thread Ville Syrjälä
On Tue, Aug 26, 2014 at 10:11:07AM +0200, Daniel Vetter wrote:
 On Tue, Aug 26, 2014 at 10:10:09AM +0200, Daniel Vetter wrote:
  On Tue, Aug 19, 2014 at 02:21:21PM -0700, clinton.a.tay...@intel.com wrote:
   From: Clint Taylor clinton.a.tay...@intel.com
   
   Pixel replicated modes should be 720 horizontal pixel 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 fizes
   the HDMI certification issues with 480i/576i.
   
   V2: Removed interlace flag from VICs 44 and 45. Will be submitted in
   another patch. Various other formatting fixes.
   
   V3: 576i@200 htotal fixed. Check min and max pixel clocks.
   
   Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
   ---
drivers/gpu/drm/drm_edid.c|   96 
   ++---
drivers/gpu/drm/i915/intel_hdmi.c |   15 --
  
  Still plea to split out the i915 change.
 
 Also it's good practice to add all the people who commented on earlier
 versions of this patch to the in-patch Cc: list. So Ville should be here
 for this one.

Also I've totally lost track of these now. Please use --in-reply-to
when sending revised patches.

-- 
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/edid: Reduce horizontal timings for pixel replicated modes

2014-09-01 Thread Ville Syrjälä
On Mon, Sep 01, 2014 at 04:12:27PM +0300, Ville Syrjälä wrote:
 On Tue, Aug 26, 2014 at 10:11:07AM +0200, Daniel Vetter wrote:
  On Tue, Aug 26, 2014 at 10:10:09AM +0200, Daniel Vetter wrote:
   On Tue, Aug 19, 2014 at 02:21:21PM -0700, clinton.a.tay...@intel.com 
   wrote:
From: Clint Taylor clinton.a.tay...@intel.com

Pixel replicated modes should be 720 horizontal pixel 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 fizes
the HDMI certification issues with 480i/576i.

V2: Removed interlace flag from VICs 44 and 45. Will be submitted in
another patch. Various other formatting fixes.

V3: 576i@200 htotal fixed. Check min and max pixel clocks.

Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
 drivers/gpu/drm/drm_edid.c|   96 
++---
 drivers/gpu/drm/i915/intel_hdmi.c |   15 --
   
   Still plea to split out the i915 change.
  
  Also it's good practice to add all the people who commented on earlier
  versions of this patch to the in-patch Cc: list. So Ville should be here
  for this one.
 
 Also I've totally lost track of these now. Please use --in-reply-to
 when sending revised patches.

Oh and please use --to --cc etc. also instead of sending the same patch
separately to multiple lists.

-- 
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/edid: Reduce horizontal timings for pixel replicated modes

2014-09-01 Thread Ville Syrjälä
On Tue, Aug 19, 2014 at 02:21:21PM -0700, clinton.a.tay...@intel.com wrote:
 From: Clint Taylor clinton.a.tay...@intel.com
 
 Pixel replicated modes should be 720 horizontal pixel 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 fizes
 the HDMI certification issues with 480i/576i.
 
 V2: Removed interlace flag from VICs 44 and 45. Will be submitted in
 another patch. Various other formatting fixes.
 
 V3: 576i@200 htotal fixed. Check min and max pixel clocks.
 
 Signed-off-by: Clint Taylor clinton.a.tay...@intel.com

Assuming you split it up like Daniel wanted you can slap
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
onto both parts.

 ---
  drivers/gpu/drm/drm_edid.c|   96 
 ++---
  drivers/gpu/drm/i915/intel_hdmi.c |   15 --
  2 files changed, 60 insertions(+), 51 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
 index f905c63..dc25999 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, 

Re: [Intel-gfx] [PATCH] drm/i915: Prevent recursive deadlock on releasing a busy userptr

2014-09-01 Thread Tvrtko Ursulin


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

Thanks,

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


Re: [Intel-gfx] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Imre Deak
On Mon, 2014-09-01 at 17:02 +1000, Dave Airlie wrote:
 From: Dave Airlie airl...@redhat.com
 Date: Mon, 1 Sep 2014 16:58:12 +1000
 Subject: [PATCH] drm/i915: handle G45/GM45 pulse detection connected
 state.
 
 In the HPD pulse handler we check for long pulses if the port is
 actually
 connected, however we do that for IBX, but we use the pulse handling
 code on
 GM45 systems as well, so we need to use a diffent check.
 
 This patch refactors the digital port connected check out of the g4x
 detection
 path and reuses it in the hpd pulse path.
 
 Should fix:
 Message-ID: 1409382202.5141.36.ca...@marge.simpson.net
 
 Reported-by: Mike Galbraith umgwanakikb...@gmail.com
 Signed-off-by: Dave Airlie airl...@redhat.com

Daniel reviewed this already, but Jani asked me to take a look, so:
Acked-by: Imre Deak imre.d...@intel.com

One thing for the future is to move ibx_digital_port_connected() to
intel_dp.c too and make its return value match that of
g4x_digital_port_connected().

--Imre

 ---
  drivers/gpu/drm/i915/intel_dp.c | 55
 +++--
  1 file changed, 37 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c
 b/drivers/gpu/drm/i915/intel_dp.c
 index 67cfed6..81d7681 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3661,24 +3661,12 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 return intel_dp_detect_dpcd(intel_dp);
  }
  
 -static enum drm_connector_status
 -g4x_dp_detect(struct intel_dp *intel_dp)
 +static int g4x_digital_port_connected(struct drm_device *dev,
 +  struct intel_digital_port
 *intel_dig_port)
  {
 -   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);
 uint32_t bit;
  
 -   /* 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;
 -   }
 -
 if (IS_VALLEYVIEW(dev)) {
 switch (intel_dig_port-port) {
 case PORT_B:
 @@ -3691,7 +3679,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
 break;
 default:
 -   return connector_status_unknown;
 +   return -EINVAL;
 }
 } else {
 switch (intel_dig_port-port) {
 @@ -3705,11 +3693,36 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
 break;
 default:
 -   return connector_status_unknown;
 +   return -EINVAL;
 }
 }
  
 if ((I915_READ(PORT_HOTPLUG_STAT)  bit) == 0)
 +   return 0;
 +   return 1;
 +}
 +
 +static enum drm_connector_status
 +g4x_dp_detect(struct intel_dp *intel_dp)
 +{
 +   struct drm_device *dev = intel_dp_to_dev(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;
 +   else if (ret == 0)
 return connector_status_disconnected;
  
 return intel_dp_detect_dpcd(intel_dp);
 @@ -4066,8 +4079,14 @@ intel_dp_hpd_pulse(struct intel_digital_port
 *intel_dig_port, bool long_hpd)
 intel_display_power_get(dev_priv, power_domain);
  
 if (long_hpd) {
 -   if (!ibx_digital_port_connected(dev_priv,
 intel_dig_port))
 -   goto mst_fail;
 +
 +   if (HAS_PCH_SPLIT(dev)) {
 +   if (!ibx_digital_port_connected(dev_priv,
 intel_dig_port))
 +   goto mst_fail;
 +   } else {
 +   if (g4x_digital_port_connected(dev,
 intel_dig_port) != 1)
 +   goto mst_fail;
 +   }
  
 if (!intel_dp_get_dpcd(intel_dp)) {
 goto mst_fail;



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list

[Intel-gfx] [PATCH 0/4] Rework workaround init fn for BDW and CHV

2014-09-01 Thread Arun Siluvery
The patches that initialize workarounds for BDW and CHV using LRIs
are already merged in the tree but I received few more comments
from Chris and Damien.

I have reworked these patches as per their comments; it fixes
some issues and the code now looks clean. we can easily add
more workarounds with minimal changes.

I have tried to split BDW and CHV changes as separate patches but
it was getting ugly hence combined them also since it is a fixup
patch it should be ok.

In the previous implementation we were emitting each LRI individually
so the total number was not clear during init so a temporary array
was used to save w/a data (this is exported to debugfs) but if not
initialized we could potentially overrun array and also the condition
was not correct.
In this rework, w/a are organized as an array so we know the exact
count from the beginning and temporary array is no longer required.

The corresponding i-g-t is gem_workarounds.c and it is also updated.

Arun Siluvery (2):
  drm/i915: Rework workaround init functions for BDW and CHV
  drm/i915: Rework workaround data exporting to debugfs

Damien Lespiau (2):
  drm/i915: Rename intel_wa_registers with a i915_ prefix
  drm/i915: Don't restrict i915_wa_registers to BDW

 drivers/gpu/drm/i915/i915_debugfs.c |  42 ++---
 drivers/gpu/drm/i915/i915_drv.h |  14 ---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 162 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |   8 ++
 4 files changed, 107 insertions(+), 119 deletions(-)

-- 
2.0.4

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


[Intel-gfx] [PATCH 1/4] drm/i915: Rework workaround init functions for BDW and CHV

2014-09-01 Thread Arun Siluvery
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
very 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.

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 149 +---
 1 file changed, 58 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c5e4dc7..bae1527 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -650,87 +650,71 @@ 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 u32 *lri_list)
 {
-   struct drm_device *dev = ring-dev;
-   struct drm_i915_private *dev_priv = dev-dev_private;
+   int i;
+   int ret;
 
-   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  2*num_registers; i += 2) {
+   intel_ring_emit(ring, *(lri_list + i));
+   intel_ring_emit(ring, *(lri_list + i + 1));
+   }
 
-   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++;
+   intel_ring_emit(ring, MI_NOOP);
+   intel_ring_advance(ring);
 
-   return;
+   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 u32 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));
+   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));
+   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));
+   HALF_SLICE_CHICKEN3,
+   _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS
+  | GEN8_SAMPLER_POWER_BYPASS_DIS),
 
-   intel_ring_emit_wa(ring, GEN7_HALF_SLICE_CHICKEN1,
-  
_MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE));
+   GEN7_HALF_SLICE_CHICKEN1,
+   _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE),
 
-   intel_ring_emit_wa(ring, COMMON_SLICE_CHICKEN2,
-  
_MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE));
+   COMMON_SLICE_CHICKEN2,
+   _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE),
 
/* Use Force Non-Coherent whenever executing a 3D context. This is a
 * workaround for for a possible hang in the 

[Intel-gfx] [PATCH 4/4] drm/i915: Rework workaround data exporting to debugfs

2014-09-01 Thread Arun Siluvery
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 | 41 +++--
 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, 52 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2727bda..bab0408 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_printf(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,27 @@ 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/2);
+   for (i = 0; i  ro_data.num_items; i += 2) {
+   u32 addr, mask, value;
+
+   addr = ro_data.init_context[i];
+   /*
+* Most of workarounds are  masked registers;
+* to set a bit in lower 16-bits we set a mask bit in
+* upper 16-bits so we can take either of them as mask but
+* it doesn't work if the w/a is about clearing a bit so
+* use upper 16-bits to cover both cases.
+*/
+   mask = ro_data.init_context[i+1]  16;
+
+   /*
+* value represents the status of other bits in the
+* register besides w/a bits
+*/
+   value  = I915_READ(addr) | mask;
+   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 bae1527..6c07e69 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -760,6 +760,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;
+}
+
 

[Intel-gfx] [PATCH 3/4] drm/i915: Don't restrict i915_wa_registers to BDW

2014-09-01 Thread Arun Siluvery
From: Damien Lespiau damien.lesp...@intel.com

We have CHV code that already makes the test obsolete. Besides, when
num_wa_regs is 0 (platforms not gathering that W/A data), we expose
something sensible already.

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Arun Siluvery arun.siluv...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index f09ba8c..2727bda 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2466,11 +2466,6 @@ static int i915_wa_registers(struct seq_file *m, void 
*unused)
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   if (!IS_BROADWELL(dev)) {
-   DRM_DEBUG_DRIVER(Workaround table not available !!\n);
-   return -EINVAL;
-   }
-
ret = mutex_lock_interruptible(dev-struct_mutex);
if (ret)
return ret;
-- 
2.0.4

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


[Intel-gfx] [PATCH 2/4] drm/i915: Rename intel_wa_registers with a i915_ prefix

2014-09-01 Thread Arun Siluvery
From: Damien Lespiau damien.lesp...@intel.com

Those debugfs files are prefixed by i915, the name of the kernel module,
presumably to make the difference with files exposed by core DRM.

Also, add a ',' at the end of the last entry. This is to ease the
conflict resolution when rebasing internal patches that add a member at
the end of the array. Without it, wiggle can't do its job as we need to
modify an existing line (appending the ',').

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index f0d63f6..f09ba8c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2458,7 +2458,7 @@ static int i915_shared_dplls_info(struct seq_file *m, 
void *unused)
return 0;
 }
 
-static int intel_wa_registers(struct seq_file *m, void *unused)
+static int i915_wa_registers(struct seq_file *m, void *unused)
 {
int i;
int ret;
@@ -4026,7 +4026,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{i915_semaphore_status, i915_semaphore_status, 0},
{i915_shared_dplls_info, i915_shared_dplls_info, 0},
{i915_dp_mst_info, i915_dp_mst_info, 0},
-   {intel_wa_registers, intel_wa_registers, 0}
+   {i915_wa_registers, i915_wa_registers, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
2.0.4

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


[Intel-gfx] [PATCH 0/2] Rework workaround igt

2014-09-01 Thread Arun Siluvery
Rework of the test as kernel patch is modified.

Arun Siluvery (1):
  igt/gem_workarounds: rework igt to test workaround registers

Damien Lespiau (1):
  gem_workarounds: intel_wa_registers is now prefixed with i915

 tests/gem_workarounds.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

-- 
2.0.4

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


[Intel-gfx] [PATCH 2/2] igt/gem_workarounds: rework igt to test workaround registers

2014-09-01 Thread Arun Siluvery
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; we can use this data to skip platform checks.

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
 tests/gem_workarounds.c | 44 +++-
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 32156d2..fae4382 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;
 
 
@@ -153,7 +153,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);
+   wa_regs[i].mask;
if (status)
++fail_count;
 
@@ -171,21 +171,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 +187,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


[Intel-gfx] [PATCH 1/2] gem_workarounds: intel_wa_registers is now prefixed with i915

2014-09-01 Thread Arun Siluvery
From: Damien Lespiau damien.lesp...@intel.com

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 tests/gem_workarounds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 6826562..32156d2 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -184,7 +184,7 @@ igt_main
devid = intel_get_drm_devid(drm_fd);
batch = intel_batchbuffer_alloc(bufmgr, devid);
 
-   fd = igt_debugfs_open(intel_wa_registers, O_RDONLY);
+   fd = igt_debugfs_open(i915_wa_registers, O_RDONLY);
igt_assert(fd = 0);
 
file = fdopen(fd, r);
-- 
2.0.4

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


Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rework workaround data exporting to debugfs

2014-09-01 Thread Damien Lespiau
On Mon, Sep 01, 2014 at 02:28:53PM +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 | 41 
 +++--
  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, 52 insertions(+), 26 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 2727bda..bab0408 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_printf(m, Workarounds applied: 0\n);

seq_puts()

 + DRM_DEBUG_DRIVER(Workaround table not available !!\n);
 + return -EINVAL;
 + }
  
   ret = mutex_lock_interruptible(dev-struct_mutex);
   if (ret)
 @@ -2472,18 +2480,27 @@ 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/2);
 + for (i = 0; i  ro_data.num_items; i += 2) {
 + u32 addr, mask, value;
 +
 + addr = ro_data.init_context[i];
 + /*
 +  * Most of workarounds are  masked registers;
 +  * to set a bit in lower 16-bits we set a mask bit in
 +  * upper 16-bits so we can take either of them as mask but
 +  * it doesn't work if the w/a is about clearing a bit so
 +  * use upper 16-bits to cover both cases.
 +  */
 + mask = ro_data.init_context[i+1]  16;

Most doesn't seem good here. Either it's all and we're happy, or we
need a generic way to describe the W/A (masked register or not). value +
mask is generic enough to code for both cases.

 +
 + /*
 +  * value represents the status of other bits in the
 +  * register besides w/a bits
 +  */
 + value  = I915_READ(addr) | mask;
 + seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n,
 +addr, value, mask);
   }

I still don't get it. 'value' is supposed to be the reference value for
the W/A, but you're or'ing the mask here, so you treat the mask as if it
were the reference value. This won't work if the W/A is about setting
multi-bits fields or about clearing a bit.

The comment is still not clear enough. You're saying other bits besides
the w/a bits, but or'ing the mask doesn't do that.

Why do we care about the other bits in the reference value? they don't
matter. Why use something else than (ro_data.init_context[i+1]  0x)
for the value here (as long we're talking about masked registers)?

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


Re: [Intel-gfx] [PATCH] drm: Include task-name and master status in debugfs clients info

2014-09-01 Thread David Herrmann
Hi

On Sat, Aug 9, 2014 at 8:22 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 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.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/drm_info.c | 17 -
  1 file changed, 12 insertions(+), 5 deletions(-)

 diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
 index 15ec9f4..d813430 100644
 --- a/drivers/gpu/drm/drm_info.c
 +++ b/drivers/gpu/drm/drm_info.c
 @@ -184,14 +184,21 @@ int drm_clients_info(struct seq_file *m, void *data)
 struct drm_file *priv;

 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,
 +   seq_printf(m,pid dev master a   uid  
 magic\n);

Maybe mention task here? Or is this supposed to be a logical part of pid?

 +   list_for_each_entry_reverse(priv, dev-filelist, lhead) {

No idea why you do backwards traversal, but ok..

 +   struct task_struct *task;
 +
 +   rcu_read_lock();

What's that rcu-lock for? task-comm is pre-allocated, priv-pid is
static, kuid... no idea?

Anyway, patch looks good otherwise. Especially task-comm sounds
really handy in that list.

Thanks
David

 +   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;
 --
 1.9.1

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


Re: [Intel-gfx] [PATCH 2/2] igt/gem_workarounds: rework igt to test workaround registers

2014-09-01 Thread Damien Lespiau
On Mon, Sep 01, 2014 at 02:29:47PM +0100, Arun Siluvery wrote:
 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; we can use this data to skip platform checks.
 
 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
 ---
  tests/gem_workarounds.c | 44 +++-
  1 file changed, 23 insertions(+), 21 deletions(-)
 
 diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
 index 32156d2..fae4382 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;
  
  
 @@ -153,7 +153,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);
 + wa_regs[i].mask;

Hum. This looks so fishy it can't be right. The heart of the problem is
that you're not clear what a mask or value should be.

To me:

  - A mask selects bits
  - value is the reference W/A value (containing only the correct bits
for a single W/A, some of those bits can be 0, mask is telling us
which bits we're interested about.
  - read(reg)  mask is the W/A value read back from the register

-- 
Damien

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


Re: [Intel-gfx] [drm/i915/3.17] panic in i915_digport_work_func

2014-09-01 Thread Jani Nikula
On Mon, 01 Sep 2014, Imre Deak imre.d...@intel.com wrote:
 On Mon, 2014-09-01 at 17:02 +1000, Dave Airlie wrote:
 From: Dave Airlie airl...@redhat.com
 Date: Mon, 1 Sep 2014 16:58:12 +1000
 Subject: [PATCH] drm/i915: handle G45/GM45 pulse detection connected
 state.
 
 In the HPD pulse handler we check for long pulses if the port is
 actually
 connected, however we do that for IBX, but we use the pulse handling
 code on
 GM45 systems as well, so we need to use a diffent check.
 
 This patch refactors the digital port connected check out of the g4x
 detection
 path and reuses it in the hpd pulse path.
 
 Should fix:
 Message-ID: 1409382202.5141.36.ca...@marge.simpson.net
 
 Reported-by: Mike Galbraith umgwanakikb...@gmail.com
 Signed-off-by: Dave Airlie airl...@redhat.com

 Daniel reviewed this already, but Jani asked me to take a look, so:
 Acked-by: Imre Deak imre.d...@intel.com

Pushed to drm-intel-fixes, thanks for the patch, review, and testing.

BR,
Jani.


 One thing for the future is to move ibx_digital_port_connected() to
 intel_dp.c too and make its return value match that of
 g4x_digital_port_connected().

 --Imre

 ---
  drivers/gpu/drm/i915/intel_dp.c | 55
 +++--
  1 file changed, 37 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c
 b/drivers/gpu/drm/i915/intel_dp.c
 index 67cfed6..81d7681 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3661,24 +3661,12 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 return intel_dp_detect_dpcd(intel_dp);
  }
  
 -static enum drm_connector_status
 -g4x_dp_detect(struct intel_dp *intel_dp)
 +static int g4x_digital_port_connected(struct drm_device *dev,
 +  struct intel_digital_port
 *intel_dig_port)
  {
 -   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);
 uint32_t bit;
  
 -   /* 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;
 -   }
 -
 if (IS_VALLEYVIEW(dev)) {
 switch (intel_dig_port-port) {
 case PORT_B:
 @@ -3691,7 +3679,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
 break;
 default:
 -   return connector_status_unknown;
 +   return -EINVAL;
 }
 } else {
 switch (intel_dig_port-port) {
 @@ -3705,11 +3693,36 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
 break;
 default:
 -   return connector_status_unknown;
 +   return -EINVAL;
 }
 }
  
 if ((I915_READ(PORT_HOTPLUG_STAT)  bit) == 0)
 +   return 0;
 +   return 1;
 +}
 +
 +static enum drm_connector_status
 +g4x_dp_detect(struct intel_dp *intel_dp)
 +{
 +   struct drm_device *dev = intel_dp_to_dev(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;
 +   else if (ret == 0)
 return connector_status_disconnected;
  
 return intel_dp_detect_dpcd(intel_dp);
 @@ -4066,8 +4079,14 @@ intel_dp_hpd_pulse(struct intel_digital_port
 *intel_dig_port, bool long_hpd)
 intel_display_power_get(dev_priv, power_domain);
  
 if (long_hpd) {
 -   if (!ibx_digital_port_connected(dev_priv,
 intel_dig_port))
 -   goto mst_fail;
 +
 +   if (HAS_PCH_SPLIT(dev)) {
 +   if (!ibx_digital_port_connected(dev_priv,
 intel_dig_port))
 +   goto mst_fail;
 +   } else {
 +   if (g4x_digital_port_connected(dev,
 intel_dig_port) != 1)
 +   goto mst_fail;
 +   }
  
 if (!intel_dp_get_dpcd(intel_dp)) {
 goto mst_fail;


-- 
Jani 

Re: [Intel-gfx] [PATCH] drm: Include task-name and master status in debugfs clients info

2014-09-01 Thread Chris Wilson
On Mon, Sep 01, 2014 at 04:11:43PM +0200, David Herrmann wrote:
 Hi
 
 On Sat, Aug 9, 2014 at 8:22 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
  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.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  ---
   drivers/gpu/drm/drm_info.c | 17 -
   1 file changed, 12 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
  index 15ec9f4..d813430 100644
  --- a/drivers/gpu/drm/drm_info.c
  +++ b/drivers/gpu/drm/drm_info.c
  @@ -184,14 +184,21 @@ int drm_clients_info(struct seq_file *m, void *data)
  struct drm_file *priv;
 
  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,
  +   seq_printf(m,pid dev master a   uid  
  magic\n);
 
 Maybe mention task here? Or is this supposed to be a logical part of pid?

Yeah, I was thinking that comm/pid were essentially the same column. Top
uses command which would be reasonable to reuse.
 
  +   list_for_each_entry_reverse(priv, dev-filelist, lhead) {
 
 No idea why you do backwards traversal, but ok..

Clients are added youngest-first. So traversing backwards gives
  kernel
  X/display manager
  clients
 
  +   struct task_struct *task;
  +
  +   rcu_read_lock();
 
 What's that rcu-lock for? task-comm is pre-allocated, priv-pid is
 static, kuid... no idea?

Cargo-culting the locking from the discussion with Tetsuo:

commit 3ec2f427e6f82b9b8f9b18dd2c758b864385df39
Author: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
Date:   Fri Jan 3 20:42:18 2014 +0900

drm/i915: Fix refcount leak and possible NULL pointerdereference.

Since get_pid_task() grabs a reference on the task_struct, we have to drop 
the
refcount after reading that task's comm name. Use pid_task() with RCU 
instead.

Also, avoid directly reading like pid_task()-comm because
pid_task() will return NULL if the task have already exit()ed.

This patch fixes both problems.


 Anyway, patch looks good otherwise. Especially task-comm sounds
 really handy in that list.

It was. Thanks for the feedback, will add the extra header and comment.
-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: Include task-name and master status in debugfs clients info

2014-09-01 Thread David Herrmann
Hi

On Mon, Sep 1, 2014 at 4:19 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Mon, Sep 01, 2014 at 04:11:43PM +0200, David Herrmann wrote:
 Hi

 On Sat, Aug 9, 2014 at 8:22 AM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
  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.
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
  ---
   drivers/gpu/drm/drm_info.c | 17 -
   1 file changed, 12 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
  index 15ec9f4..d813430 100644
  --- a/drivers/gpu/drm/drm_info.c
  +++ b/drivers/gpu/drm/drm_info.c
  @@ -184,14 +184,21 @@ int drm_clients_info(struct seq_file *m, void *data)
  struct drm_file *priv;
 
  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,
  +   seq_printf(m,pid dev master a   uid  
  magic\n);

 Maybe mention task here? Or is this supposed to be a logical part of pid?

 Yeah, I was thinking that comm/pid were essentially the same column. Top
 uses command which would be reasonable to reuse.

Sounds all fine. Just wanted to go sure it's not a typo.

  +   list_for_each_entry_reverse(priv, dev-filelist, lhead) {

 No idea why you do backwards traversal, but ok..

 Clients are added youngest-first. So traversing backwards gives
   kernel
   X/display manager
   clients

  +   struct task_struct *task;
  +
  +   rcu_read_lock();

 What's that rcu-lock for? task-comm is pre-allocated, priv-pid is
 static, kuid... no idea?

 Cargo-culting the locking from the discussion with Tetsuo:

 commit 3ec2f427e6f82b9b8f9b18dd2c758b864385df39
 Author: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
 Date:   Fri Jan 3 20:42:18 2014 +0900

 drm/i915: Fix refcount leak and possible NULL pointerdereference.

 Since get_pid_task() grabs a reference on the task_struct, we have to 
 drop the
 refcount after reading that task's comm name. Use pid_task() with RCU 
 instead.

 Also, avoid directly reading like pid_task()-comm because
 pid_task() will return NULL if the task have already exit()ed.

 This patch fixes both problems.

Ah, right, this is for pid lookup not task-comm. Should have seen that..

Feel free to add:
Reviewed-by: David Herrmann dh.herrm...@gmail.com

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


Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rework workaround data exporting to debugfs

2014-09-01 Thread Siluvery, Arun

On 01/09/2014 15:06, Damien Lespiau wrote:

On Mon, Sep 01, 2014 at 02:28:53PM +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 | 41 +++--
  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, 52 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2727bda..bab0408 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_printf(m, Workarounds applied: 0\n);


seq_puts()


+   DRM_DEBUG_DRIVER(Workaround table not available !!\n);
+   return -EINVAL;
+   }

ret = mutex_lock_interruptible(dev-struct_mutex);
if (ret)
@@ -2472,18 +2480,27 @@ 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/2);
+   for (i = 0; i  ro_data.num_items; i += 2) {
+   u32 addr, mask, value;
+
+   addr = ro_data.init_context[i];
+   /*
+* Most of workarounds are  masked registers;
+* to set a bit in lower 16-bits we set a mask bit in
+* upper 16-bits so we can take either of them as mask but
+* it doesn't work if the w/a is about clearing a bit so
+* use upper 16-bits to cover both cases.
+*/
+   mask = ro_data.init_context[i+1]  16;


Most doesn't seem good here. Either it's all and we're happy, or we
need a generic way to describe the W/A (masked register or not). value +
mask is generic enough to code for both cases.


It seems some of them could be unmasked registers.
We can use 'mask' itself to determine whether it is a masked/unmasked 
register. mask == 0 if it is an unmasked register.

+
+   /*
+* value represents the status of other bits in the
+* register besides w/a bits
+*/
+   value  = I915_READ(addr) | mask;
+   seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n,
+  addr, value, mask);
}


I still don't get it. 'value' is supposed to be the reference value for
the W/A, but you're or'ing the mask here, so you treat the mask as if it
were the reference value. This won't work if the W/A is about setting
multi-bits fields or about clearing a bit.

The comment is still not clear enough. You're saying other bits besides
the w/a bits, but or'ing the mask doesn't do that.

Why do we care about the other bits in the reference value? they don't
matter. Why use something else than (ro_data.init_context[i+1]  0x)
for the value here (as long we're talking about masked registers)?

I have always considered value as the register value (remaining bits of 
the register and w/a bits) and now I see your point.
Yes lower 16-bits can be used as reference value, depending on whether 
it is a masked/unmasked we can use/not use the mask in conjunction with 
value in the test.


regards
Arun


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


[Intel-gfx] [PATCH] drm/i915: Don't call intel_plane_restore() when the prop value didn't change

2014-09-01 Thread ville . syrjala
From: Ville Syrjälä ville.syrj...@linux.intel.com

No point in calling intel_plane_restore() in .set_property() if the
value didn't change.

More importantly this papers over a bug where the current primary plane
code forgets to update the user coordinates we store under intel_plane
unless the primary plane .update_plane() hook is actually called. This
means we have 0 in the coordinates straight after boot and any call
to intel_restore_plane() (such as from restore_fbdev_mode()) will
actually turn off the primary plane. This mess needs to be fixed properly
but that's a bigger task and the first step there is killing off
intel_pipe_set_base() and just calling the primary plane
.update_plane() hook. For the immediate problem of black screen after
boot this small patch is enough to hide it.

The problem originates from these two commits:
 commit 3a5f87c286515c54ff5c52c3e64d0c522b7570c0
 Author: Thomas Wood thomas.w...@intel.com
 Date:   Wed Aug 20 14:45:00 2014 +0100

drm: fix plane rotation when restoring fbdev configuration

 commit d91a2cb8e5104233c02bbde539bd4ee455ec12ac
 Author: Sonika Jindal sonika.jin...@intel.com
 Date:   Fri Aug 22 14:06:04 2014 +0530

drm/i915: Add 180 degree primary plane rotation support

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
---
 drivers/gpu/drm/i915/intel_sprite.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index fd5f271..cf596cd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1232,6 +1232,9 @@ int intel_plane_set_property(struct drm_plane *plane,
if (hweight32(val  0xf) != 1)
return -EINVAL;
 
+   if (intel_plane-rotation == val)
+   return 0;
+
old_val = intel_plane-rotation;
intel_plane-rotation = val;
ret = intel_plane_restore(plane);
-- 
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 call intel_plane_restore() when the prop value didn't change

2014-09-01 Thread Damien Lespiau
On Mon, Sep 01, 2014 at 06:08:25PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 No point in calling intel_plane_restore() in .set_property() if the
 value didn't change.
 
 More importantly this papers over a bug where the current primary plane
 code forgets to update the user coordinates we store under intel_plane
 unless the primary plane .update_plane() hook is actually called. This
 means we have 0 in the coordinates straight after boot and any call
 to intel_restore_plane() (such as from restore_fbdev_mode()) will
 actually turn off the primary plane. This mess needs to be fixed properly
 but that's a bigger task and the first step there is killing off
 intel_pipe_set_base() and just calling the primary plane
 .update_plane() hook. For the immediate problem of black screen after
 boot this small patch is enough to hide it.
 
 The problem originates from these two commits:
  commit 3a5f87c286515c54ff5c52c3e64d0c522b7570c0
  Author: Thomas Wood thomas.w...@intel.com
  Date:   Wed Aug 20 14:45:00 2014 +0100
 
 drm: fix plane rotation when restoring fbdev configuration
 
  commit d91a2cb8e5104233c02bbde539bd4ee455ec12ac
  Author: Sonika Jindal sonika.jin...@intel.com
  Date:   Fri Aug 22 14:06:04 2014 +0530
 
 drm/i915: Add 180 degree primary plane rotation support
 
 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

-- 
Damien

 ---
  drivers/gpu/drm/i915/intel_sprite.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 b/drivers/gpu/drm/i915/intel_sprite.c
 index fd5f271..cf596cd 100644
 --- a/drivers/gpu/drm/i915/intel_sprite.c
 +++ b/drivers/gpu/drm/i915/intel_sprite.c
 @@ -1232,6 +1232,9 @@ int intel_plane_set_property(struct drm_plane *plane,
   if (hweight32(val  0xf) != 1)
   return -EINVAL;
  
 + if (intel_plane-rotation == val)
 + return 0;
 +
   old_val = intel_plane-rotation;
   intel_plane-rotation = val;
   ret = intel_plane_restore(plane);
 -- 
 1.8.5.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] Regression in drm-intel caused by d91a2cb8e510

2014-09-01 Thread Alan Stern
Daniel:

I'm encountering a problem with the drm-intel-nightly branch in your
drm-intel repository.  The symptom is that when I boot my laptop, at
some point during the start-up procedure the screen goes totally blank,
as though the backlight were turned off, and it remains that way.  I
can't tell if the backlight really is off -- it may be that there's
just nothing being displayed -- and playing with the screen-brightness
function keys doesn't help.

Git bisect located the source of the problem as commit d91a2cb8e510
(drm/i915: Add 180 degree primary plane rotation support).  The problem
occurs when I boot a kernel built from that commit, and it doesn't
occur when I boot a kernel built from the preceding commit
(19ac737dc3c3).

If necessary, I can get debugging info by logging in over a network 
connection.  Tell me what you need.

Alan Stern

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


Re: [Intel-gfx] Regression in drm-intel caused by d91a2cb8e510

2014-09-01 Thread Damien Lespiau
On Mon, Sep 01, 2014 at 04:19:23PM -0400, Alan Stern wrote:
 Daniel:
 
 I'm encountering a problem with the drm-intel-nightly branch in your
 drm-intel repository.  The symptom is that when I boot my laptop, at
 some point during the start-up procedure the screen goes totally blank,
 as though the backlight were turned off, and it remains that way.  I
 can't tell if the backlight really is off -- it may be that there's
 just nothing being displayed -- and playing with the screen-brightness
 function keys doesn't help.
 
 Git bisect located the source of the problem as commit d91a2cb8e510
 (drm/i915: Add 180 degree primary plane rotation support).  The problem
 occurs when I boot a kernel built from that commit, and it doesn't
 occur when I boot a kernel built from the preceding commit
 (19ac737dc3c3).
 
 If necessary, I can get debugging info by logging in over a network 
 connection.  Tell me what you need.

It seems like Ville and Mika already caught a similar bug today, could
you give Ville's patch a go (available on the intel-gfx):

  drm/i915: Don't call intel_plane_restore() when the prop value didn't change


Thanks,

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


Re: [Intel-gfx] Regression in drm-intel caused by d91a2cb8e510

2014-09-01 Thread Alan Stern
On Mon, 1 Sep 2014, Damien Lespiau wrote:

 On Mon, Sep 01, 2014 at 04:19:23PM -0400, Alan Stern wrote:
  Daniel:
  
  I'm encountering a problem with the drm-intel-nightly branch in your
  drm-intel repository.  The symptom is that when I boot my laptop, at
  some point during the start-up procedure the screen goes totally blank,
  as though the backlight were turned off, and it remains that way.  I
  can't tell if the backlight really is off -- it may be that there's
  just nothing being displayed -- and playing with the screen-brightness
  function keys doesn't help.
  
  Git bisect located the source of the problem as commit d91a2cb8e510
  (drm/i915: Add 180 degree primary plane rotation support).  The problem
  occurs when I boot a kernel built from that commit, and it doesn't
  occur when I boot a kernel built from the preceding commit
  (19ac737dc3c3).
  
  If necessary, I can get debugging info by logging in over a network 
  connection.  Tell me what you need.
 
 It seems like Ville and Mika already caught a similar bug today, could
 you give Ville's patch a go (available on the intel-gfx):
 
   drm/i915: Don't call intel_plane_restore() when the prop value didn't change

Yes, that patch fixed the problem.  Thank you.

Tested-by: Alan Stern st...@rowland.harvard.edu

Alan Stern

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