[Intel-gfx] [PATCH] [fixes] drm/i915/ringbuffer: Idling requires waiting for the ring to be empty

2011-07-12 Thread Chris Wilson
...which is measured by the size and not the amount of space remaining.

Waiting upon size-8, did one of two things. In the common case with more
than 8 bytes available to write into the ring, it would return
immediately. Otherwise, it would timeout given the impossible condition
of waiting for more space than is available in the ring, leading to
warnings such as:

[drm:intel_cleanup_ring_buffer] *ERROR* failed to quiesce render ring
whilst cleaning up: -16

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/intel_ringbuffer.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c0e0ee6..39ac2b6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -165,7 +165,7 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer 
*ring);
 int __must_check intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n);
 static inline int intel_wait_ring_idle(struct intel_ring_buffer *ring)
 {
-   return intel_wait_ring_buffer(ring, ring-space - 8);
+   return intel_wait_ring_buffer(ring, ring-size - 8);
 }
 
 int __must_check intel_ring_begin(struct intel_ring_buffer *ring, int n);
-- 
1.7.5.4

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


Re: [Intel-gfx] Major 2.6.38 / 2.6.39 regression ignored?

2011-07-12 Thread Kirill Smelkov
On Sat, May 28, 2011 at 05:19:20PM +0400, Kirill Smelkov wrote:
 Hello Chris, everyone,
 
 On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote:
  On Sat, 21 May 2011 11:23:53 -0400, Luke-Jr l...@dashjr.org wrote:
   On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote:
On Fri, 20 May 2011 11:08:56 -0700, Ray Lee ray...@madrabbit.org 
wrote:
 [ Adding Chris Wilson (author of the problematic patch) and Rafael
 Wysocki to the message ]
 
 On Fri, May 20, 2011 at 10:06 AM, Luke-Jr l...@dashjr.org wrote:
  I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a 
  month
  ago against 2.6.38. Now 2.6.39 was just released without the
  regression being addressed. This bug makes the system unusable... 
  Some
  guys on IRC suggested I
  email, so here it is.
 
 See the bugzilla entry for the bisection history.

Which has nothing to do with Luke's bug. Considering the thousand things
that can go wrong during X starting, without a hint as to which it is 
nigh
on impossible to debug except by trial and error. If you set up
netconsole, does the kernel emit an OOPS with it's last dying breath?
   
   Why assume it's a different bug? I would almost wonder if it might affect 
   all Sandy Bridge GPUs. In any case, I no longer have the original 
   motherboard (it was recalled, as I said in the first post), nor even the 
   revision of it (it had other issues that weren't being fixed). I *assume* 
   I 
   will have the same problem with my new motherboard (Intel DQ67SW), but I 
   haven't verified that yet. I'll be sure to try a netconsole when I have 
   to 
   reboot next and get a chance to try the most recent 2.6.38 and .39 
   kernels, 
   but at the moment it seems reasonable to address the problem bisected in 
   the 
   bug, even if it turns out to be different.
  
  The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate
  locking between release and IRQ and so is prone to such races as befell
  Kirill should not surprise anyone. As neither UMS nor DRI supported SNB,
  I can quite confidently state they are separate bugs.
  -Chris
 
 I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok
 on kernels  2.6.38, and starting from 2.6.38 the system is just
 unusable because X either crashes the kernel (2.6.38), or does not start
 at all (2.6.39):
 
 https://bugzilla.kernel.org/show_bug.cgi?id=36052
 
 
 It's a regression. It's blocking me to upgrade to newer kernels. I've
 done my homework -- digged it and came with detailed OOPS on netconsole
 and bisected to single commit. Could this please be fixed?

Silence...

Still, reverting the bisected patch helps even for 3.0:

https://bugzilla.kernel.org/show_bug.cgi?id=36052#c4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PULL] drm-intel-fixes (drm/i915 driver)

2011-07-12 Thread Keith Packard

What have we got here: 

 * A list of DP fixes from Jesse to make the code conform more closely
   to the specification.

 * Making Ivybridge use the Sandybridge GPU reset path.

 * Recover from i915 load failure without causing a later panic
   when the shrinker ran.

 * Revert the RC6 enable patch -- there are at least two machines which
   mysteriously fail with RC6 enabled. We found lots of possible causes,
   none of which appear to help these last few hold-outs.

 * Fix an obvious typo -- the GPU idling code was using the wrong variable
   for the size of the ring. This may well cause spurious suspend
   failures as the GPU wouldn't have been reliably idled.

The following changes since commit fe0d42203cb5616eeff68b14576a0f7e2dd56625:

  Linux 3.0-rc6 (2011-07-04 15:56:24 -0700)

are available in the git repository at:
  ssh://master.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git 
drm-intel-fixes

Chris Wilson (1):
  drm/i915/ringbuffer: Idling requires waiting for the ring to be empty

Jesse Barnes (7):
  drm/i915/dp: retry link status read 3 times on failure
  drm/i915/dp: use DP DPCD defines when looking at DPCD values
  drm/i915/dp: read more receiver capability bits on hotplug
  drm/i915/dp: try to read receiver capabilities 3 times when detecting
  drm/i915/dp: remove DPMS mode tracking from DP
  drm/i915/dp: consolidate AUX retry code
  drm/i915/dp: manage sink power state if possible

Keith Packard (2):
  drm/i915: Clean up i915_driver_load failure path
  Revert drm/i915: enable rc6 by default

Kenneth Graunke (1):
  drm/i915: Enable GPU reset on Ivybridge.

 drivers/gpu/drm/i915/i915_dma.c |   14 +++-
 drivers/gpu/drm/i915/i915_drv.c |3 +-
 drivers/gpu/drm/i915/intel_dp.c |  118 ---
 drivers/gpu/drm/i915/intel_ringbuffer.h |2 +-
 4 files changed, 105 insertions(+), 32 deletions(-)


-- 
keith.pack...@intel.com


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


[Intel-gfx] [PATCH] agp/intel: Fix typo in G4x_GMCH_SIZE_VT_2M

2011-07-12 Thread Chris Wilson
Konstantin Belousov found an error in the define of G4x_GMCH_SIZE_VT_2M
relative to the GMCH specs, and confirmed that indeed one of his users
with a Q45 reports 0xb not 0xc for a 2/2MiB GATT.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Konstantin Belousov kostik...@gmail.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
--

v2: Fixed typo spotted by Konstantin.

---
 drivers/char/agp/intel-agp.h |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
index 999803c..5da67f1 100644
--- a/drivers/char/agp/intel-agp.h
+++ b/drivers/char/agp/intel-agp.h
@@ -90,9 +90,10 @@
 #define G4x_GMCH_SIZE_MASK (0xf  8)
 #define G4x_GMCH_SIZE_1M   (0x1  8)
 #define G4x_GMCH_SIZE_2M   (0x3  8)
-#define G4x_GMCH_SIZE_VT_1M(0x9  8)
-#define G4x_GMCH_SIZE_VT_1_5M  (0xa  8)
-#define G4x_GMCH_SIZE_VT_2M(0xc  8)
+#define G4x_GMCH_SIZE_VT_EN(0x8  8)
+#define G4x_GMCH_SIZE_VT_1M(G4x_GMCH_SIZE_1M | G4x_GMCH_SIZE_VT_EN)
+#define G4x_GMCH_SIZE_VT_1_5M  ((0x2  8) | G4x_GMCH_SIZE_VT_EN)
+#define G4x_GMCH_SIZE_VT_2M(G4x_GMCH_SIZE_2M | G4x_GMCH_SIZE_VT_EN)
 
 #define GFX_FLSH_CNTL  0x2170 /* 915+ */
 
-- 
1.7.5.4

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


Re: [Intel-gfx] [PATCH] agp/intel: Fix typo in G4x_GMCH_SIZE_VT_2M

2011-07-12 Thread Chris Wilson
On Fri, 8 Jul 2011 13:56:36 +0300, Konstantin Belousov kostik...@gmail.com 
wrote:
Non-text part: multipart/signed
 On Fri, Jul 08, 2011 at 11:48:50AM +0100, Chris Wilson wrote:
  +#define G4x_GMCH_SIZE_VT_1_5M  (0x2 | G4x_GMCH_SIZE_VT_EN) /* no nonVT 
  equiv */
 I think this should be (0x2  8), and not 0x2.

Absolutely right.
-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] agp/intel: Fix typo in G4x_GMCH_SIZE_VT_2M

2011-07-12 Thread Daniel Vetter
Bleh, I need to fix my in-brain hex calculator ;-)

Acked-by: Daniel Vetter daniel.vet...@ffwll.ch
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 0/7] Minor DisplayPort cleanup

2011-07-12 Thread Adam Jackson
Patch 1 is actually core drm, and independent of the rest of the series.
The rest vary among cosmetic cleanup, unifying the G4X/IRL paths, and
one corner-case change to be more strictly spec-compliant.

Tested on a Thinkpad T500 (GM45) with various DisplayPort attachments.
My X200 (with DP only on the dock) is still not especially reliable, but
I'm sure I'll figure something out.

- ajax

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


[Intel-gfx] [PATCH 2/7] drm/i915/dp: Zero the DPCD data before connection probe

2011-07-12 Thread Adam Jackson
Signed-off-by: Adam Jackson a...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2aced6..de24f31 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1673,6 +1673,7 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
struct edid *edid = NULL;
 
intel_dp-has_audio = false;
+   memset(intel_dp-dpcd, 0, sizeof(intel_dp-dpcd));
 
if (HAS_PCH_SPLIT(dev))
status = ironlake_dp_detect(intel_dp);
-- 
1.7.6

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


[Intel-gfx] [PATCH 3/7] drm/i915/dp: Move DPCD dump to common code instead of PCH-only

2011-07-12 Thread Adam Jackson
No reason not to see this on g4x, after all.

Signed-off-by: Adam Jackson a...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index de24f31..0be85a0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1615,8 +1615,6 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 sizeof (intel_dp-dpcd));
if (ret  intel_dp-dpcd[DP_DPCD_REV] != 0)
status = connector_status_connected;
-   DRM_DEBUG_KMS(DPCD: %hx%hx%hx%hx\n, intel_dp-dpcd[0],
- intel_dp-dpcd[1], intel_dp-dpcd[2], intel_dp-dpcd[3]);
return status;
 }
 
@@ -1679,6 +1677,10 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
status = ironlake_dp_detect(intel_dp);
else
status = g4x_dp_detect(intel_dp);
+
+   DRM_DEBUG_KMS(DPCD: %hx%hx%hx%hx\n, intel_dp-dpcd[0],
+ intel_dp-dpcd[1], intel_dp-dpcd[2], intel_dp-dpcd[3]);
+
if (status != connector_status_connected)
return status;
 
-- 
1.7.6

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


[Intel-gfx] [PATCH 4/7] drm/i915/dp: Read more DPCD registers on connection probe

2011-07-12 Thread Adam Jackson
For parity with radeon and nouveau, and also because I suspect we're
going to need it to get format-conversion dongles right.

Signed-off-by: Adam Jackson a...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0be85a0..2f0566b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -52,7 +52,7 @@ struct intel_dp {
uint32_t color_range;
uint8_t link_bw;
uint8_t lane_count;
-   uint8_t dpcd[4];
+   uint8_t dpcd[8];
struct i2c_adapter adapter;
struct i2c_algo_dp_aux_data algo;
bool is_pch_edp;
@@ -1678,8 +1678,10 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
else
status = g4x_dp_detect(intel_dp);
 
-   DRM_DEBUG_KMS(DPCD: %hx%hx%hx%hx\n, intel_dp-dpcd[0],
- intel_dp-dpcd[1], intel_dp-dpcd[2], intel_dp-dpcd[3]);
+   DRM_DEBUG_KMS(DPCD: %hx%hx%hx%hx%hx%hx%hx%hx\n, intel_dp-dpcd[0],
+ intel_dp-dpcd[1], intel_dp-dpcd[2], intel_dp-dpcd[3],
+ intel_dp-dpcd[4], intel_dp-dpcd[5], intel_dp-dpcd[6],
+ intel_dp-dpcd[7]);
 
if (status != connector_status_connected)
return status;
-- 
1.7.6

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


[Intel-gfx] [PATCH 5/7] drm/i915/dp: Better hexdump of DPCD

2011-07-12 Thread Adam Jackson
%hx alone prints 0 as 0, not 00.

Signed-off-by: Adam Jackson a...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2f0566b..13bddd3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1678,10 +1678,10 @@ intel_dp_detect(struct drm_connector *connector, bool 
force)
else
status = g4x_dp_detect(intel_dp);
 
-   DRM_DEBUG_KMS(DPCD: %hx%hx%hx%hx%hx%hx%hx%hx\n, intel_dp-dpcd[0],
- intel_dp-dpcd[1], intel_dp-dpcd[2], intel_dp-dpcd[3],
- intel_dp-dpcd[4], intel_dp-dpcd[5], intel_dp-dpcd[6],
- intel_dp-dpcd[7]);
+   DRM_DEBUG_KMS(DPCD: %02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx\n,
+ intel_dp-dpcd[0], intel_dp-dpcd[1], intel_dp-dpcd[2],
+ intel_dp-dpcd[3], intel_dp-dpcd[4], intel_dp-dpcd[5],
+ intel_dp-dpcd[6], intel_dp-dpcd[7]);
 
if (status != connector_status_connected)
return status;
-- 
1.7.6

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


[Intel-gfx] [PATCH 7/7] drm/i915/dp: Explicitly request 8/10 channel coding

2011-07-12 Thread Adam Jackson
It's not clear what a sink would do if you wrote zero to this register -
which I guess would mean I don't support any channel encodings, good
luck - but let's not find out.

Signed-off-by: Adam Jackson a...@redhat.com
---
 drivers/gpu/drm/i915/intel_dp.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9a0c3ca..1c3a36f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -769,6 +769,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct 
drm_display_mode *mode,
memset(intel_dp-link_configuration, 0, DP_LINK_CONFIGURATION_SIZE);
intel_dp-link_configuration[0] = intel_dp-link_bw;
intel_dp-link_configuration[1] = intel_dp-lane_count;
+   intel_dp-link_configuration[8] = DP_SET_ANSI_8B10B;
 
/*
 * Check for DPCD version  1.1 and enhanced framing support
-- 
1.7.6

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


Re: [Intel-gfx] [PATCH] [fixes] drm/i915/ringbuffer: Idling requires waiting for the ring to be empty

2011-07-12 Thread Ben Widawsky

On Tue, 12 Jul 2011 18:03:29 +0100, Chris Wilson wrote:
...which is measured by the size and not the amount of space 
remaining.


Waiting upon size-8, did one of two things. In the common case with 
more

than 8 bytes available to write into the ring, it would return
immediately. Otherwise, it would timeout given the impossible 
condition

of waiting for more space than is available in the ring, leading to
warnings such as:

[drm:intel_cleanup_ring_buffer] *ERROR* failed to quiesce render ring
whilst cleaning up: -16

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


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

This may potentially fix:
https://bugzilla.kernel.org/show_bug.cgi?id=38332
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] i915: Fix opregion notifications

2011-07-12 Thread Matthew Garrett
opregion-based platforms will send ACPI video event 0x80 for a range of
notification types for legacy compatibility. This is interpreted as a
display switch event, which may not be appropriate in the circumstances.
When we receive such an event we should make sure that the platform is
genuinely requesting a display switch before passing that event through
to userspace.

Signed-off-by: Matthew Garrett m...@redhat.com
Tested-by: Adam Jackson a...@redhat.com
---
 drivers/acpi/video.c  |7 ---
 drivers/gpu/drm/i915/intel_opregion.c |   15 +++
 include/acpi/video.h  |2 ++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index db39e9e..ada4b4d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -46,7 +46,6 @@
 
 #define PREFIX ACPI: 
 
-#define ACPI_VIDEO_CLASS   video
 #define ACPI_VIDEO_BUS_NAMEVideo Bus
 #define ACPI_VIDEO_DEVICE_NAME Video Device
 #define ACPI_VIDEO_NOTIFY_SWITCH   0x80
@@ -1445,7 +1444,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
 * most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
-   keycode = KEY_SWITCHVIDEOMODE;
+   if (!acpi_notifier_call_chain(device, event, 0))
+   keycode = KEY_SWITCHVIDEOMODE;
break;
 
case ACPI_VIDEO_NOTIFY_PROBE:   /* User plugged in or removed a video
@@ -1475,7 +1475,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}
 
-   acpi_notifier_call_chain(device, event, 0);
+   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+   acpi_notifier_call_chain(device, event, 0);
 
if (keycode) {
input_report_key(input, keycode, 1);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index d2c7104..3443fc1c 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -297,19 +297,26 @@ static int intel_opregion_video_event(struct 
notifier_block *nb,
/* The only video events relevant to opregion are 0x80. These indicate
   either a docking event, lid switch or display switch request. In
   Linux, these are handled by the dock, button and video drivers.
-  We might want to fix the video driver to be opregion-aware in
-  future, but right now we just indicate to the firmware that the
-  request has been handled */
+   */
 
struct opregion_acpi *acpi;
+   struct acpi_bus_event *event = data;
+   int ret = NOTIFY_OK;
+
+   if (strcmp(event-device_class, ACPI_VIDEO_CLASS))
+   return NOTIFY_DONE;
 
if (!system_opregion)
return NOTIFY_DONE;
 
acpi = system_opregion-acpi;
+
+   if (event-type == 0x80  !(acpi-cevt  0x1))
+   ret = NOTIFY_BAD;
+
acpi-csts = 0;
 
-   return NOTIFY_OK;
+   return ret;
 }
 
 static struct notifier_block intel_opregion_notifier = {
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 0e98e67..61109f2 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -5,6 +5,8 @@
 
 struct acpi_device;
 
+#define ACPI_VIDEO_CLASS   video
+
 #define ACPI_VIDEO_DISPLAY_CRT  1
 #define ACPI_VIDEO_DISPLAY_TV   2
 #define ACPI_VIDEO_DISPLAY_DVI  3
-- 
1.7.6

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


Re: [Intel-gfx] [PATCH] i915: Fix opregion notifications

2011-07-12 Thread Keith Packard
On Tue, 12 Jul 2011 17:51:36 -0400, Matthew Garrett m...@redhat.com wrote:

 - keycode = KEY_SWITCHVIDEOMODE;
 + if (!acpi_notifier_call_chain(device, event, 0))
 + keycode = KEY_SWITCHVIDEOMODE;

Right, acpi_notify_call_chain returns -EINVAL when the underlying
function call returns NOTIFY_BAD.

 - acpi_notifier_call_chain(device, event, 0);
 + if (event != ACPI_VIDEO_NOTIFY_SWITCH)
 + acpi_notifier_call_chain(device, event, 0);

Not ideal to have two calls to this function only one of which will be
used. Could look like

boolcheck_call_chain = false;

...
case ACPI_VIDEO_NOTIFY_SWITCH:
acpi_bus_generate_proc_event(device, event, 0);
keycode = KEY_SWITCHVIDEOMODE;
check_call_chain = true
break;
...

if (acpi_notifier_call_chain(device, event, 0)  0)
if (check_call_chain)
keycode = 0;

Feel free to just call me for bikeshedding.

 + if (strcmp(event-device_class, ACPI_VIDEO_CLASS))
 + return NOTIFY_DONE;

I'm not entirely clear on accepted coding style here, but I'd much
rather this look like:

if (strcmp(event-device_class, ACPI_VIDEO_CLASS) != 0)

I must have read this four times before I figured out that you weren't
early returning on all video devices...
 
 +
 + if (event-type == 0x80  !(acpi-cevt  0x1))
 + ret = NOTIFY_BAD;
 +

Seriously? It's some kind of magic non-switch switch event? And you can
tell by checking magic bits within the event?

How can I test this and know if it works?

-- 
keith.pack...@intel.com


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


[Intel-gfx] [PATCH V2] i915: Fix opregion notifications

2011-07-12 Thread Matthew Garrett
opregion-based platforms will send ACPI video event 0x80 for a range of
notification types for legacy compatibility. This is interpreted as a
display switch event, which may not be appropriate in the circumstances.
When we receive such an event we should make sure that the platform is
genuinely requesting a display switch before passing that event through
to userspace.

Signed-off-by: Matthew Garrett m...@redhat.com
Tested-by: Adam Jackson a...@redhat.com
---
 drivers/acpi/video.c  |7 ---
 drivers/gpu/drm/i915/intel_opregion.c |   15 +++
 include/acpi/video.h  |2 ++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index db39e9e..ada4b4d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -46,7 +46,6 @@
 
 #define PREFIX ACPI: 
 
-#define ACPI_VIDEO_CLASS   video
 #define ACPI_VIDEO_BUS_NAMEVideo Bus
 #define ACPI_VIDEO_DEVICE_NAME Video Device
 #define ACPI_VIDEO_NOTIFY_SWITCH   0x80
@@ -1445,7 +1444,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH:  /* User requested a switch,
 * most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
-   keycode = KEY_SWITCHVIDEOMODE;
+   if (!acpi_notifier_call_chain(device, event, 0))
+   keycode = KEY_SWITCHVIDEOMODE;
break;
 
case ACPI_VIDEO_NOTIFY_PROBE:   /* User plugged in or removed a video
@@ -1475,7 +1475,8 @@ static void acpi_video_bus_notify(struct acpi_device 
*device, u32 event)
break;
}
 
-   acpi_notifier_call_chain(device, event, 0);
+   if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+   acpi_notifier_call_chain(device, event, 0);
 
if (keycode) {
input_report_key(input, keycode, 1);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index d2c7104..b7c5ddb 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -297,19 +297,26 @@ static int intel_opregion_video_event(struct 
notifier_block *nb,
/* The only video events relevant to opregion are 0x80. These indicate
   either a docking event, lid switch or display switch request. In
   Linux, these are handled by the dock, button and video drivers.
-  We might want to fix the video driver to be opregion-aware in
-  future, but right now we just indicate to the firmware that the
-  request has been handled */
+   */
 
struct opregion_acpi *acpi;
+   struct acpi_bus_event *event = data;
+   int ret = NOTIFY_OK;
+
+   if (strcmp(event-device_class, ACPI_VIDEO_CLASS) != 0)
+   return NOTIFY_DONE;
 
if (!system_opregion)
return NOTIFY_DONE;
 
acpi = system_opregion-acpi;
+
+   if (event-type == 0x80  !(acpi-cevt  0x1))
+   ret = NOTIFY_BAD;
+
acpi-csts = 0;
 
-   return NOTIFY_OK;
+   return ret;
 }
 
 static struct notifier_block intel_opregion_notifier = {
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 0e98e67..61109f2 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -5,6 +5,8 @@
 
 struct acpi_device;
 
+#define ACPI_VIDEO_CLASS   video
+
 #define ACPI_VIDEO_DISPLAY_CRT  1
 #define ACPI_VIDEO_DISPLAY_TV   2
 #define ACPI_VIDEO_DISPLAY_DVI  3
-- 
1.7.6

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


Re: [Intel-gfx] [PATCH 1/7] drm: Fill in more of the DisplayPort DPCD registers

2011-07-12 Thread Keith Packard
On Tue, 12 Jul 2011 17:37:59 -0400, Adam Jackson a...@redhat.com wrote:
 Signed-off-by: Adam Jackson a...@redhat.com

 +# define DP_DWN_STRM_PORT_TYPE_TMDS  (2  1)

This could be labeled TYPE_DVI_OR_HDMI according to the DP 1.1a spec.

 -

I find the blank lines useful in this file; they separate lists of
values for different fields.


 -#define DP_INTERLANE_ALIGN_DONE  (1  0)
 -#define DP_DOWNSTREAM_PORT_STATUS_CHANGED   (1  6)
 -#define DP_LINK_STATUS_UPDATED   (1  7)
 +# define DP_INTERLANE_ALIGN_DONE (1  0)
 +# define DP_DOWNSTREAM_PORT_STATUS_CHANGED  (1  6)
 +# define DP_LINK_STATUS_UPDATED  (1  7)
  
  #define DP_SINK_STATUS   0x205
 -
 -#define DP_RECEIVE_PORT_0_STATUS (1  0)
 -#define DP_RECEIVE_PORT_1_STATUS (1  1)
 +# define DP_RECEIVE_PORT_0_STATUS(1  0)
 +# define DP_RECEIVE_PORT_1_STATUS(1  1)
  
  #define DP_ADJUST_REQUEST_LANE0_10x206
  #define DP_ADJUST_REQUEST_LANE2_30x207

Please make a separate patch for whitespace cleanups...

Otherwise, these changes all match the DP 1.1a spec that I compared them
with.

Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


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


Re: [Intel-gfx] [PATCH 7/7] drm/i915/dp: Explicitly request 8/10 channel coding

2011-07-12 Thread Keith Packard

Patches 2-7:

Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


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


Re: [Intel-gfx] [PATCH] i915: Fix opregion notifications

2011-07-12 Thread Matthew Garrett
On Tue, Jul 12, 2011 at 03:20:23PM -0700, Keith Packard wrote:

 Seriously? It's some kind of magic non-switch switch event? And you can
 tell by checking magic bits within the event?

The Intel drivers on Windows appear to have used 0x80 as a generic 
Something's chanegd notification, overloading it for docking, lid 
state change and display switch press. As a result of that right now 
we're sending a display switch event whenever one of these occurs. Less 
than ideal. Opregion gives us a magic field in shared OS/firmware memory 
that can be used to distinguish between these events.

 How can I test this and know if it works?

Tested on a Thinkpad X200. Make sure suspend is disabled, then close the 
lid. Open it again. The ACPI video input device will send a display 
switch event. Add this patch and repeat and the event will vanish. The 
display switch itself will still generate an event.

I'll send v2 now.

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx