Re: [Intel-gfx] [PATCH] xf86-video-intel: Unbreak build on OpenBSD

2014-02-18 Thread Chris Wilson
On Sat, Feb 15, 2014 at 09:29:42PM +0100, Mark Kettenis wrote:
 Fallout from the backlight helper changes.

Apologies for missing this earlier. I have rearranged the code once
again to try to push the OpenBSD specifics down into src/backlight.c,
can you please check what needs to be fixed up now?
-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 4/5] drm/i915: Add rotation support for the cursor plane

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 01:19:05PM +0530, Sagar Arun Kamble wrote:
 On Mon, 2014-02-17 at 19:51 +0200, Ville Syrjälä wrote:
  On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
   On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
On Fri, Feb 14, 2014 at 04:31:17PM +0530, Sagar Arun Kamble wrote:
 On Wed, 2014-02-12 at 23:15 +0200, ville.syrj...@linux.intel.com 
 wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  The cursor plane also supports 180 degree rotation. Add a new
  cursor-rotation property on the crtc which controls this.
  
  Unlike sprites, the cursor has a fixed size, so if you have a small
  cursor image with the rest of the bo filled by transparent pixels,
  simply flipping the rotation property will cause the visible part
  of the cursor to shift. This is something to keep in mind when
  using cursor rotation.
 By flipping you meant setting 180 degree rotation?

Yes.

 Don't we have to adjust the cursor base as well to the lower right
 corner apart from setting the control bit?

No, the hardware does that automagically. Hmm. Except on gen4
apparently. Looks like I need to test on gen4, and fix it if it's
really the case.
   I tried on BYT system and 180 rotation on cursor plane is showing
   garbage data in cursor plane. We might need to adjust the cursor base.
  
  Yeah it's the same on gen4. I already have a fixed patch, but didn't
  repost it yet.
  
   Another thing, pipe rotation somehow did not work for me when I do this:
   echo 0x4  /sys/kernel/debug/dri/0/i915_pipe_rotation
   Only cursor plane had impact. Need to debug this as well.
  
  That's expected. It doesn't actually call the set_property codepath,
  instead it just sets the value directly and excpects a subsequent
  modeset to do the actual work. It was anyway just a hack to try things
  out a bit, so I didn't implement it properly. But it should be trivial
  to make it work correctly, so I might as well do it...
 Yeah. Tried doing modeset and it works perfectly.
 For Cursor rotation we might need to add check for 32bpp cursors as
 well.

We don't support anything else at the moment. And I don't think there's
much point in adding support for any legacy cursor formats. The one
thing we want to do is add support for larger cursor sizes. But I think
that can wait until we expose the cursor as a drm_plane.

-- 
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 4/5] drm/i915: Add rotation support for the cursor plane

2014-02-18 Thread Sagar Arun Kamble
On Tue, 2014-02-18 at 11:23 +0200, Ville Syrjälä wrote:
 On Tue, Feb 18, 2014 at 01:19:05PM +0530, Sagar Arun Kamble wrote:
  On Mon, 2014-02-17 at 19:51 +0200, Ville Syrjälä wrote:
   On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
 On Fri, Feb 14, 2014 at 04:31:17PM +0530, Sagar Arun Kamble wrote:
  On Wed, 2014-02-12 at 23:15 +0200, ville.syrj...@linux.intel.com 
  wrote:
   From: Ville Syrjälä ville.syrj...@linux.intel.com
   
   The cursor plane also supports 180 degree rotation. Add a new
   cursor-rotation property on the crtc which controls this.
   
   Unlike sprites, the cursor has a fixed size, so if you have a 
   small
   cursor image with the rest of the bo filled by transparent pixels,
   simply flipping the rotation property will cause the visible part
   of the cursor to shift. This is something to keep in mind when
   using cursor rotation.
  By flipping you meant setting 180 degree rotation?
 
 Yes.
 
  Don't we have to adjust the cursor base as well to the lower right
  corner apart from setting the control bit?
 
 No, the hardware does that automagically. Hmm. Except on gen4
 apparently. Looks like I need to test on gen4, and fix it if it's
 really the case.
I tried on BYT system and 180 rotation on cursor plane is showing
garbage data in cursor plane. We might need to adjust the cursor base.
   
   Yeah it's the same on gen4. I already have a fixed patch, but didn't
   repost it yet.
   
Another thing, pipe rotation somehow did not work for me when I do this:
echo 0x4  /sys/kernel/debug/dri/0/i915_pipe_rotation
Only cursor plane had impact. Need to debug this as well.
   
   That's expected. It doesn't actually call the set_property codepath,
   instead it just sets the value directly and excpects a subsequent
   modeset to do the actual work. It was anyway just a hack to try things
   out a bit, so I didn't implement it properly. But it should be trivial
   to make it work correctly, so I might as well do it...
  Yeah. Tried doing modeset and it works perfectly.
  For Cursor rotation we might need to add check for 32bpp cursors as
  well.
 
 We don't support anything else at the moment. And I don't think there's
 much point in adding support for any legacy cursor formats. The one
 thing we want to do is add support for larger cursor sizes. But I think
 that can wait until we expose the cursor as a drm_plane.
Actually I am working on enabling larger cursor sizes now. Will share
patch now.
 


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


[Intel-gfx] [PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

2014-02-18 Thread sagar . a . kamble
From: Sagar Kamble sagar.a.kam...@intel.com

With this patch we allow larger cursor planes of sizes 128x128
and 256x256. Planning to extend kms_cursor_crc test for verifying
these larger planes.

Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Cc: David Airlie airl...@linux.ie
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: G, Pallavi pallav...@intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 
 drivers/gpu/drm/i915/intel_display.c | 25 -
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2f564ce..2fee3a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3522,7 +3522,11 @@
 /* New style CUR*CNTR flags */
 #define   CURSOR_MODE  0x27
 #define   CURSOR_MODE_DISABLE   0x00
+#define   CURSOR_MODE_128_32B_AX 0x02
+#define   CURSOR_MODE_256_32B_AX 0x03
 #define   CURSOR_MODE_64_32B_AX 0x07
+#define   CURSOR_MODE_128_ARGB_AX ((1  5) | CURSOR_MODE_128_32B_AX)
+#define   CURSOR_MODE_256_ARGB_AX ((1  5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1  5) | CURSOR_MODE_64_32B_AX)
 #define   MCURSOR_PIPE_SELECT  (1  28)
 #define   MCURSOR_PIPE_A   0x00
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f19e6ea..00b51f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
u32 base)
bool visible = base != 0;
 
if (intel_crtc-cursor_visible != visible) {
+   int16_t width = intel_crtc-cursor_width;
uint32_t cntl = I915_READ(CURCNTR(pipe));
if (base) {
cntl = ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-   cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+   if (width == 64)
+   cntl |= CURSOR_MODE_64_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
+   else if (width == 128)
+   cntl |= CURSOR_MODE_128_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
+   else if (width == 256)
+   cntl |= CURSOR_MODE_256_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
+
cntl |= pipe  28; /* Connect to correct pipe */
} else {
cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
@@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, 
u32 base)
bool visible = base != 0;
 
if (intel_crtc-cursor_visible != visible) {
+   int16_t width = intel_crtc-cursor_width;
uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
if (base) {
cntl = ~CURSOR_MODE;
-   cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
+
+   if (width == 64)
+   cntl |= CURSOR_MODE_64_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
+   else if (width == 128)
+   cntl |= CURSOR_MODE_128_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
+   else if (width == 256)
+   cntl |= CURSOR_MODE_256_ARGB_AX | 
MCURSOR_GAMMA_ENABLE;
} else {
cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
cntl |= CURSOR_MODE_DISABLE;
@@ -7538,9 +7553,9 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
goto finish;
}
 
-   /* Currently we only support 64x64 cursors */
-   if (width != 64 || height != 64) {
-   DRM_ERROR(we currently only support 64x64 cursors\n);
+   /* Check for which cursor types we support */
+   if (width  256 || height  256) {
+   DRM_ERROR(We currently only support 64x64, 128x128, 256x256 
cursors\n);
return -EINVAL;
}
 
-- 
1.8.5

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


Re: [Intel-gfx] [PATCH v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Mika Westerberg
On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
 + {
 + .callback = video_set_use_native_backlight,
 + .ident = HP EliteBook 2013 models,
 + .matches = {
 + DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
 + DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
 + DMI_MATCH(DMI_PRODUCT_NAME,  G1),
 + },
 + },

I see my device is listed here but the above doesn't really use native
backlight because it is still in acpi_osi blacklist. Tried this and I can
see both acpi_video0 and intel_backlight listed under /sys/class/backlight.
Was this the intention?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Added a new Panel-fitter property for connectors

2014-02-18 Thread akash . goel
From: Akash Goel akash.g...@intel.com

Added a Panel-fitter enable/disable property support for the
connectors.

NOTE: By default, the value for the property will be disabled.

Signed-off-by: G Pallavi pallav...@intel.com
Signed-off-by: Akash Goel akash.g...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  8 
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 31 +++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c64831..a07f6cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1560,6 +1560,7 @@ typedef struct drm_i915_private {
 
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+   struct drm_property *force_pfit_property;
 
uint32_t hw_context_size;
struct list_head context_list;
@@ -1613,6 +1614,13 @@ enum hdmi_force_audio {
HDMI_AUDIO_ON,  /* force turn on HDMI audio */
 };
 
+enum panel_fitter {
+   PFIT_OFF,
+   AUTOSCALE,
+   PILLARBOX,
+   LETTERBOX,
+};
+
 #define I915_GTT_OFFSET_NONE ((u32)-1)
 
 struct drm_i915_gem_object_ops {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4ffc02..db6ea3e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -819,6 +819,7 @@ int intel_connector_update_modes(struct drm_connector 
*connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_force_pfit_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c 
b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..967e080 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -28,6 +28,7 @@
 #include linux/fb.h
 #include drm/drm_edid.h
 #include drm/drmP.h
+#include drm/drm_crtc.h
 #include intel_drv.h
 #include i915_drv.h
 
@@ -126,3 +127,33 @@ intel_attach_broadcast_rgb_property(struct drm_connector 
*connector)
 
drm_object_attach_property(connector-base, prop, 0);
 }
+
+static const struct drm_prop_enum_list pfit_names[] = {
+   { PFIT_OFF, pfit off },
+   { AUTO_SCALE, Auto scale },
+   { PILLAR_BOX, PillarBox },
+   { LETTER_BOX, LetterBox },
+};
+
+void
+intel_attach_force_pfit_property(struct drm_connector *connector)
+{
+   struct drm_device *dev = connector-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct drm_property *prop;
+   struct drm_mode_object *obj = connector-base;
+
+   prop = dev_priv-force_pfit_property;
+   if (prop == NULL) {
+   prop = drm_property_create_enum(dev, 0,
+   pfit,
+   pfit_names,
+   ARRAY_SIZE(pfit_names));
+   if (prop == NULL)
+   return;
+
+   dev_priv-force_pfit_property = prop;
+   }
+
+   drm_object_attach_property(obj, prop, 0);
+}
-- 
1.8.5.2

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


[Intel-gfx] [PATCH 9/5] drm/i915: Draw a picture about video timings

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

The docs are a bit lacking when it comes to describing when certain
timing related events occur in the hardware. Draw a picture which
tries to capture the most important ones.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bec9af8..6fa8693 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -625,6 +625,54 @@ i915_pipe_enabled(struct drm_device *dev, int pipe)
}
 }
 
+/*
+ * This timing diagram depicts the video signal in and
+ * around the vertical blanking period.
+ *
+ * Assumptions about the fictitious mode used in this example:
+ *  vblank_start = 3
+ *  vsync_start = vblank_start + 1
+ *  vsync_end = vblank_start + 2
+ *  vtotal = vblank_start + 3
+ *
+ *   start of vblank:
+ *   latch double buffered registers
+ *   increment frame counter (ctg+)
+ *   generate start of vblank interrupt (gen4+)
+ *   |
+ *   |  frame start:
+ *   |  generate frame start interrupt (aka. vblank interrupt) 
(gmch)
+ *   |  may be shifted forward 1-3 extra lines via PIPECONF
+ *   |  |
+ *   |  |  start of vsync:
+ *   |  |  generate vsync interrupt
+ *   |  |  |
+ * _________________________________
+ *   .   \hs/   .  \hs/  \hs/  \hs/   .  \hs/
+ * va--- -vb va-
+ *   |  |   vs- |
+ * -vbs- ---vbs+1--- ---vbs+2--- -0- -1- ---2- 
(scanline gen2)
+ * -vbs-2--- ---vbs-1--- ---vbs- ---vbs+1--- ---vbs+2--- ---0- 
(scanline gen3+)
+ *   |  | |
+ *   last visible pixel   first visible 
pixel
+ *  | increment frame 
counter (gen3/4)
+ *  pixel counter = vblank_start * htotal pixel counter = 
0 (gen3/4)
+ *
+ * x  = horizontal active
+ * _  = horizontal blanking
+ * hs = horizontal sync
+ * va = vertical active
+ * vb = vertical blanking
+ * vs = vertical sync
+ * vbs = vblank_start (number)
+ *
+ * Summary:
+ * - most events happen at the start of horizontal sync
+ * - frame start happens at the start of horizontal blank
+ * - gen3/4 pixel and frame counter are synchronized with the start
+ *   of horizontal active on the first line of vertical active
+ */
+
 static u32 i8xx_get_vblank_counter(struct drm_device *dev, int pipe)
 {
/* Gen2 doesn't have a hardware frame counter */
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 8/5] drm/i915: Fix gen2 scanline counter

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

On gen2 the scanline counter behaves a bit differently from the
later generations. Instead of adding one to the raw scanline
counter value, we must subtract one.

I've not yet verified which way gen3 works. My suspicion is that
it behaves like gen4 since for the most part the other timing
related registers are similar to gen4. The one similarity to gen2
that gen3 shares is the lack of the start of vblank interrupt.
The event is there internally even on gen2 (that's where the
registers get latched) but the PIPESTAT status bit and the actual
interrupt are not available to the driver.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fc49fb6..bec9af8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -724,26 +724,36 @@ static int __intel_get_crtc_scanline(struct intel_crtc 
*crtc)
struct drm_i915_private *dev_priv = dev-dev_private;
const struct drm_display_mode *mode = crtc-config.adjusted_mode;
enum pipe pipe = crtc-pipe;
-   int vtotal = mode-crtc_vtotal;
-   int position;
+   int position, vtotal;
 
+   vtotal = mode-crtc_vtotal;
if (mode-flags  DRM_MODE_FLAG_INTERLACE)
vtotal /= 2;
 
+   /*
+* The scanline counter increments at the leading edge of hsync.
+*
+* On most platforms it starts counting from vtotal-1 on the
+* first active line. That means the scanline counter value is
+* always one less than what we would expect. Ie. just after
+* start of vblank, which also occurs at start of hsync (on the
+* last active line), the scanline counter will read vblank_start-1.
+*
+* Gen2 is the exception as the scanline counter starts counting
+* from 1 instead of vtotal-1, so we have to subtract one (or
+* rather add vtotal-1 to keep the value positive), instead of
+* adding one.
+*
+* FIXME which way does gen3 work? Gen4 is for sure in the +1 camp.
+*/
if (IS_GEN2(dev))
-   position = __raw_i915_read32(dev_priv, PIPEDSL(pipe))  
DSL_LINEMASK_GEN2;
+   position = (__raw_i915_read32(dev_priv, PIPEDSL(pipe)) 
+   DSL_LINEMASK_GEN2) + vtotal - 1;
else
-   position = __raw_i915_read32(dev_priv, PIPEDSL(pipe))  
DSL_LINEMASK_GEN3;
+   position = (__raw_i915_read32(dev_priv, PIPEDSL(pipe)) 
+   DSL_LINEMASK_GEN3) + 1;
 
-   /*
-* Scanline counter increments at leading edge of hsync, and
-* it starts counting from vtotal-1 on the first active line.
-* That means the scanline counter value is always one less
-* than what we would expect. Ie. just after start of vblank,
-* which also occurs at start of hsync (on the last active line),
-* the scanline counter will read vblank_start-1.
-*/
-   return (position + 1) % vtotal;
+   return position % vtotal;
 }
 
 static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
-- 
1.8.3.2

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


Re: [Intel-gfx] [PATCH 1/1] drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 03:39:46PM +0530, sagar.a.kam...@intel.com wrote:
 From: Sagar Kamble sagar.a.kam...@intel.com
 
 With this patch we allow larger cursor planes of sizes 128x128
 and 256x256. Planning to extend kms_cursor_crc test for verifying
 these larger planes.
 
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Cc: David Airlie airl...@linux.ie
 Cc: dri-de...@lists.freedesktop.org
 Cc: linux-ker...@vger.kernel.org

Let's not spam everyone. Just intel-gfx is enough for such patches.
Well, maybe keep dri-devel too since cursor size seems to be a hot
topic across other drivers currently.

 Signed-off-by: G, Pallavi pallav...@intel.com
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h  |  4 
  drivers/gpu/drm/i915/intel_display.c | 25 -
  2 files changed, 24 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 2f564ce..2fee3a2 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -3522,7 +3522,11 @@
  /* New style CUR*CNTR flags */
  #define   CURSOR_MODE0x27
  #define   CURSOR_MODE_DISABLE   0x00
 +#define   CURSOR_MODE_128_32B_AX 0x02
 +#define   CURSOR_MODE_256_32B_AX 0x03
  #define   CURSOR_MODE_64_32B_AX 0x07
 +#define   CURSOR_MODE_128_ARGB_AX ((1  5) | CURSOR_MODE_128_32B_AX)
 +#define   CURSOR_MODE_256_ARGB_AX ((1  5) | CURSOR_MODE_256_32B_AX)
  #define   CURSOR_MODE_64_ARGB_AX ((1  5) | CURSOR_MODE_64_32B_AX)
  #define   MCURSOR_PIPE_SELECT(1  28)
  #define   MCURSOR_PIPE_A 0x00
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index f19e6ea..00b51f3 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7411,10 +7411,18 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
 u32 base)
   bool visible = base != 0;
  
   if (intel_crtc-cursor_visible != visible) {
 + int16_t width = intel_crtc-cursor_width;
   uint32_t cntl = I915_READ(CURCNTR(pipe));
   if (base) {
   cntl = ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
 - cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
 +
 + if (width == 64)
 + cntl |= CURSOR_MODE_64_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
 + else if (width == 128)
 + cntl |= CURSOR_MODE_128_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
 + else if (width == 256)
 + cntl |= CURSOR_MODE_256_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
 +
   cntl |= pipe  28; /* Connect to correct pipe */
   } else {
   cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
 @@ -7439,10 +7447,17 @@ static void ivb_update_cursor(struct drm_crtc *crtc, 
 u32 base)
   bool visible = base != 0;
  
   if (intel_crtc-cursor_visible != visible) {
 + int16_t width = intel_crtc-cursor_width;
   uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
   if (base) {
   cntl = ~CURSOR_MODE;
 - cntl |= CURSOR_MODE_64_ARGB_AX | MCURSOR_GAMMA_ENABLE;
 +
 + if (width == 64)
 + cntl |= CURSOR_MODE_64_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
 + else if (width == 128)
 + cntl |= CURSOR_MODE_128_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
 + else if (width == 256)
 + cntl |= CURSOR_MODE_256_ARGB_AX | 
 MCURSOR_GAMMA_ENABLE;
   } else {
   cntl = ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
   cntl |= CURSOR_MODE_DISABLE;
 @@ -7538,9 +7553,9 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
   goto finish;
   }
  
 - /* Currently we only support 64x64 cursors */
 - if (width != 64 || height != 64) {
 - DRM_ERROR(we currently only support 64x64 cursors\n);
 + /* Check for which cursor types we support */
 + if (width  256 || height  256) {

This has to check explicitly for 64x64, 128x128, or 256x256. Any other
combination of width and height is illegal.

Additionally gen2 supports only 64x64, so that has to be checked also.

Another issue is how to tell userspace about the cursor size. There was
a patch on dri-devel recently adding cursor width/height capability
queries to drm. I guess that should be enough for xorg since AFAICS it
only uses a single fixed cursor size. The downside is that if the actual
cursor image would fit one of the smaller sizes, we end up doing needless
memory fetches for the extra pixels.

Once we have drm_planes for cursors, I was thinking we might add some kind
of enum property that lists all the supported sizes for the plane.

 

[Intel-gfx] [PATCH] Update intel_chipset macros

2014-02-18 Thread joao . santos
From: Joao Santos joao.san...@intel.com

Added in new macros to support new platforms.

Signed-off-by: Joao Santos joao.san...@intel.com
---
 intel/intel_chipset.h |   49 +++--
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index e5589be..f7771a7 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -314,12 +314,49 @@
 
 #define IS_GEN8(devid) IS_BROADWELL(devid)
 
-#define IS_9XX(dev)(IS_GEN3(dev) || \
-IS_GEN4(dev) || \
-IS_GEN5(dev) || \
-IS_GEN6(dev) || \
-IS_GEN7(dev) || \
-IS_GEN8(dev))
+#define IS_965(devid)  (IS_GEN4(devid) || \
+IS_GEN5(devid) || \
+IS_GEN6(devid) || \
+IS_GEN7(devid) || \
+IS_GEN8(devid))
+
+#define IS_9XX(devid)  (IS_GEN3(devid) || \
+IS_GEN4(devid) || \
+IS_GEN5(devid) || \
+IS_GEN6(devid) || \
+IS_GEN7(devid) || \
+IS_GEN8(devid))
+
+#define IS_INTEL(devid)(IS_GEN2(devid) || \
+IS_GEN3(devid) || \
+IS_GEN4(devid) || \
+IS_GEN5(devid) || \
+IS_GEN6(devid) || \
+IS_GEN7(devid) || \
+IS_GEN8(devid))
+
+#define HAS_PCH_SPLIT(devid)   (IS_GEN5(devid) || \
+IS_GEN6(devid) || \
+IS_IVYBRIDGE(devid) || IS_HASWELL(devid) || \
+IS_GEN8(devid))
+
+#define HAS_BLT_RING(devid)(IS_GEN6(devid) || \
+IS_GEN7(devid) || \
+IS_GEN8(devid))
+
+#define HAS_BSD_RING(devid)(IS_GEN5(devid) || \
+IS_GEN6(devid) || \
+IS_GEN7(devid) || \
+IS_GEN8(devid))
+
+#define IS_BROADWATER(devid)   ((devid) == PCI_CHIP_I946_GZ || \
+(devid) == PCI_CHIP_I965_G_1 || \
+(devid) == PCI_CHIP_I965_Q || \
+(devid) == PCI_CHIP_I965_G)
+
+#define IS_CRESTLINE(devid)((devid) == PCI_CHIP_I965_GM || \
+(devid) == PCI_CHIP_I965_GME)
 
+#define HAS_VEBOX_RING(devid)   (IS_HASWELL(devid))
 
 #endif /* _INTEL_CHIPSET_H */
-- 
1.7.9.5

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


[Intel-gfx] [PATCH] build: Skip kms_plane and kms_sysfs_edid_timing on Android

2014-02-18 Thread joao . santos
From: Joao Santos joao.san...@intel.com

We do not have GLib there so it does not build.

Signed-off-by: Joao Santos joao.san...@intel.com
---
 tests/Android.mk |2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/Android.mk b/tests/Android.mk
index 30be4a6..fa01721 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -32,6 +32,8 @@ endef
 ##
 
 skip_tests_list := \
+kms_plane \
+kms_sysfs_edid_timing \
 testdisplay \
 kms_addfb \
 kms_cursor_crc \
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Takashi Iwai
At Tue, 18 Feb 2014 12:34:42 +0200,
Mika Westerberg wrote:
 
 On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
  +   {
  +   .callback = video_set_use_native_backlight,
  +   .ident = HP EliteBook 2013 models,
  +   .matches = {
  +   DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
  +   DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
  +   DMI_MATCH(DMI_PRODUCT_NAME,  G1),
  +   },
  +   },
 
 I see my device is listed here but the above doesn't really use native
 backlight because it is still in acpi_osi blacklist. Tried this and I can
 see both acpi_video0 and intel_backlight listed under /sys/class/backlight.
 Was this the intention?

The acpi_osi blacklist commit (2d4054d84224) has to be reverted, as I
mentioned earlier.  But the revert can be done individually after
merging this patch.  Rafael, could you care?


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


Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update

2014-02-18 Thread Lin, Mengdong
Sorry to pick up this thread after a long time.

 -Original Message-
 From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
 Daniel Vetter
 Sent: Thursday, January 23, 2014 4:27 PM
 To: Takashi Iwai
 
 On Thu, Jan 23, 2014 at 8:57 AM, Takashi Iwai ti...@suse.de wrote:
  Thanks for clarification!
  Maybe we can add output info (eg. display port number) to the eld entries
 under /proc/asound/cardx. Is it okay?
 
  It's possible, but the proc file is just a help.  It can't be the API.
  For accessing the information, we'll need some new API, or let inform
  via sysfs of the new device.
 
 Links in sysfs sound like the best approach. drm already has nodes for each
 connector, so on the gfx side there's a natural endpoint already.
 sysfs links also avoids any naming issues from the start, e.g. the above DP
 connector id might lead to clashes with multiple cards.

Hi Daniel,

Is there a 1:1 mapping between these connector nodes and ports of Gfx display 
engine?
Eg. For Haswell Ultrabook, under /sys/devices/pci:00/:00:02.0/drm/card0/
There are four connector nodes, 
card0-DP-1  - DDI port B
card0-eDP-1/- DDI port A
card0-HDMI-A-1/ - DDI Port C
card0-HDMI-A-2/ - Which DDI port ?  Haswell-ULT does not support port D, and I 
think port E is for VGA.

Hi Takashi,

To make user space figure out which audio output is connected to which screen 
(connector), maybe we can define a new ALSA control for each HDMI/DP PCM device:
e.g. numid=x,iface=PCM,name='Screen',device=3
Reading the control will return the name of the DRM connector nodes like ' 
card0-DP-1'. The audio driver can get the connector name from the gfx driver.

For DP1.2 Multi-stream transport, it's not supported by i915 and HD-A driver 
now. But probably there will be sub-nodes for the DP connector node in the 
future and an index in their name can be used distinguish monitors connected to 
the same DP port, like card0-DP-1.1, card0-DP-1.2, card0-DP-1.3 ... These names 
can be used by the above ALSA PCM 'Screen' control, so we can still know which 
audio output is to which monitor.

Thanks
Mengdong

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


Re: [Intel-gfx] [PATCH 7/5] drm/i915: Improve gen3/4 frame counter

2014-02-18 Thread Imre Deak
On Tue, 2014-02-18 at 14:04 +0200, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@linux.intel.com
 
 Currently the logic to fix up the frame counter on gen3/4 assumes that
 start of vblank occurs at vblank_start*htotal pixels, when in fact
 it occurs htotal-hsync_start pixels earlier. Apply the appropriate
 adjustment to make the frame counter more accurate.
 
 Also fix the vblank start position for interlaced display modes.
 
 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_irq.c | 21 -
  1 file changed, 16 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 9f1c449..fc49fb6 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -639,7 +639,7 @@ static u32 i915_get_vblank_counter(struct drm_device 
 *dev, int pipe)
   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
   unsigned long high_frame;
   unsigned long low_frame;
 - u32 high1, high2, low, pixel, vbl_start;
 + u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
  
   if (!i915_pipe_enabled(dev, pipe)) {
   DRM_DEBUG_DRIVER(trying to get vblank count for disabled 
 @@ -653,17 +653,28 @@ static u32 i915_get_vblank_counter(struct drm_device 
 *dev, int pipe)
   const struct drm_display_mode *mode =
   intel_crtc-config.adjusted_mode;
  
 - vbl_start = mode-crtc_vblank_start * mode-crtc_htotal;
 + htotal = mode-crtc_htotal;
 + hsync_start = mode-crtc_hsync_start;
 + vbl_start = mode-crtc_vblank_start;
 + if (mode-flags  DRM_MODE_FLAG_INTERLACE)
 + vbl_start = DIV_ROUND_UP(vbl_start, 2);

The adjustment for interlace mode is already done in drm_mode_setcrtc,
so I think we don't need it here. Otherwise this patch makes sense to me
based on the signal chart you drew on this, so:

Reviewed-by: Imre Deak imre.d...@intel.com

   } else {
   enum transcoder cpu_transcoder = (enum transcoder) pipe;
 - u32 htotal;
  
   htotal = ((I915_READ(HTOTAL(cpu_transcoder))  16)  0x1fff) + 
 1;
 + hsync_start = (I915_READ(HSYNC(cpu_transcoder))   0x1fff) + 1;
   vbl_start = (I915_READ(VBLANK(cpu_transcoder))  0x1fff) + 1;
 -
 - vbl_start *= htotal;
 + if ((I915_READ(PIPECONF(cpu_transcoder)) 
 +  PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
 + vbl_start = DIV_ROUND_UP(vbl_start, 2);
   }
  
 + /* Convert to pixel count */
 + vbl_start *= htotal;
 +
 + /* Start of vblank event occurs at start of hsync */
 + vbl_start -= htotal - hsync_start;
 +
   high_frame = PIPEFRAME(pipe);
   low_frame = PIPEFRAMEPIXEL(pipe);
  



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


Re: [Intel-gfx] [PATCH] build: Skip kms_plane and kms_sysfs_edid_timing on Android

2014-02-18 Thread Damien Lespiau
On Tue, Feb 18, 2014 at 01:28:54PM +, joao.san...@intel.com wrote:
 From: Joao Santos joao.san...@intel.com
 
 We do not have GLib there so it does not build.

 
 Signed-off-by: Joao Santos joao.san...@intel.com
 ---
  tests/Android.mk |2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/tests/Android.mk b/tests/Android.mk
 index 30be4a6..fa01721 100644
 --- a/tests/Android.mk
 +++ b/tests/Android.mk
 @@ -32,6 +32,8 @@ endef
  ##
  
  skip_tests_list := \
 +kms_plane \
 +kms_sysfs_edid_timing \

None of them use glib. Oh! there were spurious glib.h includes, that
explains it, I've pushed a couple of patches to remove them.

kms_plane uses cairo which I understand you don't have (yet! :).

What about kms_sysfs_edid_timing though? it's shell script that makes
sure we don't take an abnormal time to read the EDID from a monitor.
What's the problem you're seeing?

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


Re: [Intel-gfx] Need your advice: Add a new communication inteface between HD-Audio and Gfx drivers for hotplug notification/ELD update

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 01:58:22PM +, Lin, Mengdong wrote:
 Sorry to pick up this thread after a long time.
 
  -Original Message-
  From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
  Daniel Vetter
  Sent: Thursday, January 23, 2014 4:27 PM
  To: Takashi Iwai
  
  On Thu, Jan 23, 2014 at 8:57 AM, Takashi Iwai ti...@suse.de wrote:
   Thanks for clarification!
   Maybe we can add output info (eg. display port number) to the eld entries
  under /proc/asound/cardx. Is it okay?
  
   It's possible, but the proc file is just a help.  It can't be the API.
   For accessing the information, we'll need some new API, or let inform
   via sysfs of the new device.
  
  Links in sysfs sound like the best approach. drm already has nodes for each
  connector, so on the gfx side there's a natural endpoint already.
  sysfs links also avoids any naming issues from the start, e.g. the above DP
  connector id might lead to clashes with multiple cards.
 
 Hi Daniel,
 
 Is there a 1:1 mapping between these connector nodes and ports of Gfx display 
 engine?
 Eg. For Haswell Ultrabook, under 
 /sys/devices/pci:00/:00:02.0/drm/card0/
 There are four connector nodes, 
 card0-DP-1- DDI port B
 card0-eDP-1/  - DDI port A
 card0-HDMI-A-1/   - DDI Port C
 card0-HDMI-A-2/   - Which DDI port ?  Haswell-ULT does not support port 
 D, and I think port E is for VGA.

There's no fixed mapping with the port and the connector name. The
number in the connector name is basically just a running number per
connector type. However I do believe we do register the connectors
in the order of the ports more or less always, so you can
*sometimes* deduce the port name from the connector.

I suppose in this example HDMI-A-1 is port B, HDMI-A-2 is port C, and
DP-1 can be either port B or port C. DP++ is the reason why we have
overlapping DP and HDMI connectors for the same port.

 
 Hi Takashi,
 
 To make user space figure out which audio output is connected to which screen 
 (connector), maybe we can define a new ALSA control for each HDMI/DP PCM 
 device:
 e.g. numid=x,iface=PCM,name='Screen',device=3
 Reading the control will return the name of the DRM connector nodes like ' 
 card0-DP-1'. The audio driver can get the connector name from the gfx driver.
 
 For DP1.2 Multi-stream transport, it's not supported by i915 and HD-A driver 
 now. But probably there will be sub-nodes for the DP connector node in the 
 future and an index in their name can be used distinguish monitors connected 
 to the same DP port, like card0-DP-1.1, card0-DP-1.2, card0-DP-1.3 ... These 
 names can be used by the above ALSA PCM 'Screen' control, so we can still 
 know which audio output is to which monitor.
 
 Thanks
 Mengdong
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] Update intel_chipset macros

2014-02-18 Thread Damien Lespiau
On Tue, Feb 18, 2014 at 01:15:12PM +, joao.san...@intel.com wrote:
 From: Joao Santos joao.san...@intel.com
 
 Added in new macros to support new platforms.

What's the real purpose of this patch? you're adding a bunch of defines
here without any user. I'd argue that we really want to move away from
those defines, they are quite dreadful.

If the purpose is to make libdrm's and intel-gpu-tools' intel_chipset.h
look alike, a worthy goal for sure, I'd vote for splitting the i-g-t
specific defines into a separate file in i-g-t, waiting for someone
fed-up enough with the current situation to create a more structured way
to store per-platform feature flags/limits.

-- 
Damien

 
 Signed-off-by: Joao Santos joao.san...@intel.com
 ---
  intel/intel_chipset.h |   49 
 +++--
  1 file changed, 43 insertions(+), 6 deletions(-)
 
 diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
 index e5589be..f7771a7 100644
 --- a/intel/intel_chipset.h
 +++ b/intel/intel_chipset.h
 @@ -314,12 +314,49 @@
  
  #define IS_GEN8(devid)   IS_BROADWELL(devid)
  
 -#define IS_9XX(dev)  (IS_GEN3(dev) || \
 -  IS_GEN4(dev) || \
 -  IS_GEN5(dev) || \
 -  IS_GEN6(dev) || \
 -  IS_GEN7(dev) || \
 -  IS_GEN8(dev))
 +#define IS_965(devid)(IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_9XX(devid)(IS_GEN3(devid) || \
 +  IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_INTEL(devid)  (IS_GEN2(devid) || \
 +  IS_GEN3(devid) || \
 +  IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_PCH_SPLIT(devid) (IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_IVYBRIDGE(devid) || IS_HASWELL(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_BLT_RING(devid)  (IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_BSD_RING(devid)  (IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_BROADWATER(devid) ((devid) == PCI_CHIP_I946_GZ || \
 +  (devid) == PCI_CHIP_I965_G_1 || \
 +  (devid) == PCI_CHIP_I965_Q || \
 +  (devid) == PCI_CHIP_I965_G)
 +
 +#define IS_CRESTLINE(devid)  ((devid) == PCI_CHIP_I965_GM || \
 +  (devid) == PCI_CHIP_I965_GME)
  
 +#define HAS_VEBOX_RING(devid)   (IS_HASWELL(devid))
  
  #endif /* _INTEL_CHIPSET_H */
 -- 
 1.7.9.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


Re: [Intel-gfx] [PATCH 7/5] drm/i915: Improve gen3/4 frame counter

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 04:16:00PM +0200, Imre Deak wrote:
 On Tue, 2014-02-18 at 14:04 +0200, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Currently the logic to fix up the frame counter on gen3/4 assumes that
  start of vblank occurs at vblank_start*htotal pixels, when in fact
  it occurs htotal-hsync_start pixels earlier. Apply the appropriate
  adjustment to make the frame counter more accurate.
  
  Also fix the vblank start position for interlaced display modes.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_irq.c | 21 -
   1 file changed, 16 insertions(+), 5 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_irq.c 
  b/drivers/gpu/drm/i915/i915_irq.c
  index 9f1c449..fc49fb6 100644
  --- a/drivers/gpu/drm/i915/i915_irq.c
  +++ b/drivers/gpu/drm/i915/i915_irq.c
  @@ -639,7 +639,7 @@ static u32 i915_get_vblank_counter(struct drm_device 
  *dev, int pipe)
  drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
  unsigned long high_frame;
  unsigned long low_frame;
  -   u32 high1, high2, low, pixel, vbl_start;
  +   u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
   
  if (!i915_pipe_enabled(dev, pipe)) {
  DRM_DEBUG_DRIVER(trying to get vblank count for disabled 
  @@ -653,17 +653,28 @@ static u32 i915_get_vblank_counter(struct drm_device 
  *dev, int pipe)
  const struct drm_display_mode *mode =
  intel_crtc-config.adjusted_mode;
   
  -   vbl_start = mode-crtc_vblank_start * mode-crtc_htotal;
  +   htotal = mode-crtc_htotal;
  +   hsync_start = mode-crtc_hsync_start;
  +   vbl_start = mode-crtc_vblank_start;
  +   if (mode-flags  DRM_MODE_FLAG_INTERLACE)
  +   vbl_start = DIV_ROUND_UP(vbl_start, 2);
 
 The adjustment for interlace mode is already done in drm_mode_setcrtc,
 so I think we don't need it here.

We throw away the values filled in by drm_mode_setcrtc(). Which is a
good thing since it rounds the result the wrong way (for our hardware
at least). The values we see here are filled in by
intel_modeset_pipe_config() which doesn't pass the
CRTC_INTERLACE_HALVE_V flag to drm_mode_set_crtcinfo().

 Otherwise this patch makes sense to me
 based on the signal chart you drew on this, so:
 
 Reviewed-by: Imre Deak imre.d...@intel.com
 
  } else {
  enum transcoder cpu_transcoder = (enum transcoder) pipe;
  -   u32 htotal;
   
  htotal = ((I915_READ(HTOTAL(cpu_transcoder))  16)  0x1fff) + 
  1;
  +   hsync_start = (I915_READ(HSYNC(cpu_transcoder))   0x1fff) + 1;
  vbl_start = (I915_READ(VBLANK(cpu_transcoder))  0x1fff) + 1;
  -
  -   vbl_start *= htotal;
  +   if ((I915_READ(PIPECONF(cpu_transcoder)) 
  +PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
  +   vbl_start = DIV_ROUND_UP(vbl_start, 2);
  }
   
  +   /* Convert to pixel count */
  +   vbl_start *= htotal;
  +
  +   /* Start of vblank event occurs at start of hsync */
  +   vbl_start -= htotal - hsync_start;
  +
  high_frame = PIPEFRAME(pipe);
  low_frame = PIPEFRAMEPIXEL(pipe);
   
 



-- 
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 v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Rafael J. Wysocki
On Tuesday, February 18, 2014 02:31:46 PM Takashi Iwai wrote:
 At Tue, 18 Feb 2014 12:34:42 +0200,
 Mika Westerberg wrote:
  
  On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
   + {
   + .callback = video_set_use_native_backlight,
   + .ident = HP EliteBook 2013 models,
   + .matches = {
   + DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
   + DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
   + DMI_MATCH(DMI_PRODUCT_NAME,  G1),
   + },
   + },
  
  I see my device is listed here but the above doesn't really use native
  backlight because it is still in acpi_osi blacklist. Tried this and I can
  see both acpi_video0 and intel_backlight listed under /sys/class/backlight.
  Was this the intention?
 
 The acpi_osi blacklist commit (2d4054d84224) has to be reverted, as I
 mentioned earlier.  But the revert can be done individually after
 merging this patch.  Rafael, could you care?

Done.  Please check the result in linux-pm.git/linux-next.

Thanks,
Rafael

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


Re: [Intel-gfx] [PATCH 7/5] drm/i915: Improve gen3/4 frame counter

2014-02-18 Thread Imre Deak
On Tue, 2014-02-18 at 16:41 +0200, Ville Syrjälä wrote:
 On Tue, Feb 18, 2014 at 04:16:00PM +0200, Imre Deak wrote:
  On Tue, 2014-02-18 at 14:04 +0200, ville.syrj...@linux.intel.com wrote:
   From: Ville Syrjälä ville.syrj...@linux.intel.com
   
   Currently the logic to fix up the frame counter on gen3/4 assumes that
   start of vblank occurs at vblank_start*htotal pixels, when in fact
   it occurs htotal-hsync_start pixels earlier. Apply the appropriate
   adjustment to make the frame counter more accurate.
   
   Also fix the vblank start position for interlaced display modes.
   
   Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
   ---
drivers/gpu/drm/i915/i915_irq.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/i915_irq.c 
   b/drivers/gpu/drm/i915/i915_irq.c
   index 9f1c449..fc49fb6 100644
   --- a/drivers/gpu/drm/i915/i915_irq.c
   +++ b/drivers/gpu/drm/i915/i915_irq.c
   @@ -639,7 +639,7 @@ static u32 i915_get_vblank_counter(struct drm_device 
   *dev, int pipe)
 drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
 unsigned long high_frame;
 unsigned long low_frame;
   - u32 high1, high2, low, pixel, vbl_start;
   + u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;

 if (!i915_pipe_enabled(dev, pipe)) {
 DRM_DEBUG_DRIVER(trying to get vblank count for disabled 
   @@ -653,17 +653,28 @@ static u32 i915_get_vblank_counter(struct 
   drm_device *dev, int pipe)
 const struct drm_display_mode *mode =
 intel_crtc-config.adjusted_mode;

   - vbl_start = mode-crtc_vblank_start * mode-crtc_htotal;
   + htotal = mode-crtc_htotal;
   + hsync_start = mode-crtc_hsync_start;
   + vbl_start = mode-crtc_vblank_start;
   + if (mode-flags  DRM_MODE_FLAG_INTERLACE)
   + vbl_start = DIV_ROUND_UP(vbl_start, 2);
  
  The adjustment for interlace mode is already done in drm_mode_setcrtc,
  so I think we don't need it here.
 
 We throw away the values filled in by drm_mode_setcrtc(). Which is a
 good thing since it rounds the result the wrong way (for our hardware
 at least). The values we see here are filled in by
 intel_modeset_pipe_config() which doesn't pass the
 CRTC_INTERLACE_HALVE_V flag to drm_mode_set_crtcinfo().

Ah, true, I see now. This looks also ok then.

--Imre

  Otherwise this patch makes sense to me
  based on the signal chart you drew on this, so:
  
  Reviewed-by: Imre Deak imre.d...@intel.com
  
 } else {
 enum transcoder cpu_transcoder = (enum transcoder) pipe;
   - u32 htotal;

 htotal = ((I915_READ(HTOTAL(cpu_transcoder))  16)  0x1fff) + 
   1;
   + hsync_start = (I915_READ(HSYNC(cpu_transcoder))   0x1fff) + 1;
 vbl_start = (I915_READ(VBLANK(cpu_transcoder))  0x1fff) + 1;
   -
   - vbl_start *= htotal;
   + if ((I915_READ(PIPECONF(cpu_transcoder)) 
   +  PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
   + vbl_start = DIV_ROUND_UP(vbl_start, 2);
 }

   + /* Convert to pixel count */
   + vbl_start *= htotal;
   +
   + /* Start of vblank event occurs at start of hsync */
   + vbl_start -= htotal - hsync_start;
   +
 high_frame = PIPEFRAME(pipe);
 low_frame = PIPEFRAMEPIXEL(pipe);

  
 
 
 



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


[Intel-gfx] [PATCH] drm/i915: fix forcewake counts for gen8

2014-02-18 Thread Mika Kuoppala
Generic driver code gets forcewake explicitly by gen6_gt_force_wake_get(),
which keeps force wake counts before accessing low level fw get.
However the underlying gen8 register write function access low level
accessors directly. This leads to nested fw get from hardware, causing
forcewake ack clear errors and/or hangs.

Fix this by checking the forcewake count in gen8 accessors.
Also implement read accessors for gen8 to gain symmetry for
shadowed register access.

References: https://bugs.freedesktop.org/show_bug.cgi?id=74007
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Cc: Ben Widawsky benjamin.widaw...@intel.com
---
 drivers/gpu/drm/i915/intel_uncore.c |   74 +++
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index c628414..089feaa 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -498,6 +498,45 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, 
bool trace) { \
REG_READ_FOOTER; \
 }
 
+static const u32 gen8_shadowed_regs[] = {
+   FORCEWAKE_MT,
+   GEN6_RPNSWREQ,
+   GEN6_RC_VIDEO_FREQ,
+   RING_TAIL(RENDER_RING_BASE),
+   RING_TAIL(GEN6_BSD_RING_BASE),
+   RING_TAIL(VEBOX_RING_BASE),
+   RING_TAIL(BLT_RING_BASE),
+   /* TODO: Other registers are not yet used */
+};
+
+static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg)
+{
+   int i;
+   for (i = 0; i  ARRAY_SIZE(gen8_shadowed_regs); i++)
+   if (reg == gen8_shadowed_regs[i])
+   return true;
+
+   return false;
+}
+
+#define __gen8_read(x) \
+static u##x \
+gen8_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+   bool __needs_put = reg  0x4  !is_gen8_shadowed(dev_priv, reg); \
+   REG_READ_HEADER(x); \
+   __needs_put = dev_priv-uncore.forcewake_count == 0; \
+   if (__needs_put) { \
+   dev_priv-uncore.funcs.force_wake_get(dev_priv, \
+   FORCEWAKE_ALL); \
+   } \
+   val = __raw_i915_read##x(dev_priv, reg); \
+   if (__needs_put) { \
+   dev_priv-uncore.funcs.force_wake_put(dev_priv, \
+   FORCEWAKE_ALL); \
+   } \
+   REG_READ_FOOTER; \
+}
+
 #define __vlv_read(x) \
 static u##x \
 vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
@@ -531,6 +570,10 @@ __vlv_read(8)
 __vlv_read(16)
 __vlv_read(32)
 __vlv_read(64)
+__gen8_read(8)
+__gen8_read(16)
+__gen8_read(32)
+__gen8_read(64)
 __gen6_read(8)
 __gen6_read(16)
 __gen6_read(32)
@@ -545,6 +588,7 @@ __gen4_read(32)
 __gen4_read(64)
 
 #undef __vlv_read
+#undef __gen8_read
 #undef __gen6_read
 #undef __gen5_read
 #undef __gen4_read
@@ -610,32 +654,12 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t 
reg, u##x val, bool trace)
REG_WRITE_FOOTER; \
 }
 
-static const u32 gen8_shadowed_regs[] = {
-   FORCEWAKE_MT,
-   GEN6_RPNSWREQ,
-   GEN6_RC_VIDEO_FREQ,
-   RING_TAIL(RENDER_RING_BASE),
-   RING_TAIL(GEN6_BSD_RING_BASE),
-   RING_TAIL(VEBOX_RING_BASE),
-   RING_TAIL(BLT_RING_BASE),
-   /* TODO: Other registers are not yet used */
-};
-
-static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg)
-{
-   int i;
-   for (i = 0; i  ARRAY_SIZE(gen8_shadowed_regs); i++)
-   if (reg == gen8_shadowed_regs[i])
-   return true;
-
-   return false;
-}
-
 #define __gen8_write(x) \
 static void \
 gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
trace) { \
bool __needs_put = reg  0x4  !is_gen8_shadowed(dev_priv, reg); \
REG_WRITE_HEADER; \
+   __needs_put = dev_priv-uncore.forcewake_count == 0; \
if (__needs_put) { \
dev_priv-uncore.funcs.force_wake_get(dev_priv, \
FORCEWAKE_ALL); \
@@ -734,10 +758,10 @@ void intel_uncore_init(struct drm_device *dev)
dev_priv-uncore.funcs.mmio_writew  = gen8_write16;
dev_priv-uncore.funcs.mmio_writel  = gen8_write32;
dev_priv-uncore.funcs.mmio_writeq  = gen8_write64;
-   dev_priv-uncore.funcs.mmio_readb  = gen6_read8;
-   dev_priv-uncore.funcs.mmio_readw  = gen6_read16;
-   dev_priv-uncore.funcs.mmio_readl  = gen6_read32;
-   dev_priv-uncore.funcs.mmio_readq  = gen6_read64;
+   dev_priv-uncore.funcs.mmio_readb  = gen8_read8;
+   dev_priv-uncore.funcs.mmio_readw  = gen8_read16;
+   dev_priv-uncore.funcs.mmio_readl  = gen8_read32;
+   dev_priv-uncore.funcs.mmio_readq  = gen8_read64;
break;
case 7:
case 6:
-- 
1.7.9.5

___
Intel-gfx mailing 

Re: [Intel-gfx] [PATCH] drm/i915: fix forcewake counts for gen8

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 05:48:54PM +0200, Mika Kuoppala wrote:
 Generic driver code gets forcewake explicitly by gen6_gt_force_wake_get(),
 which keeps force wake counts before accessing low level fw get.
 However the underlying gen8 register write function access low level
 accessors directly. This leads to nested fw get from hardware, causing
 forcewake ack clear errors and/or hangs.
 
 Fix this by checking the forcewake count in gen8 accessors.
 Also implement read accessors for gen8 to gain symmetry for
 shadowed register access.
 
 References: https://bugs.freedesktop.org/show_bug.cgi?id=74007
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 ---
  drivers/gpu/drm/i915/intel_uncore.c |   74 
 +++
  1 file changed, 49 insertions(+), 25 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
 b/drivers/gpu/drm/i915/intel_uncore.c
 index c628414..089feaa 100644
 --- a/drivers/gpu/drm/i915/intel_uncore.c
 +++ b/drivers/gpu/drm/i915/intel_uncore.c
 @@ -498,6 +498,45 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t 
 reg, bool trace) { \
   REG_READ_FOOTER; \
  }
  
 +static const u32 gen8_shadowed_regs[] = {
 + FORCEWAKE_MT,
 + GEN6_RPNSWREQ,
 + GEN6_RC_VIDEO_FREQ,
 + RING_TAIL(RENDER_RING_BASE),
 + RING_TAIL(GEN6_BSD_RING_BASE),
 + RING_TAIL(VEBOX_RING_BASE),
 + RING_TAIL(BLT_RING_BASE),
 + /* TODO: Other registers are not yet used */
 +};
 +
 +static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg)
 +{
 + int i;
 + for (i = 0; i  ARRAY_SIZE(gen8_shadowed_regs); i++)
 + if (reg == gen8_shadowed_regs[i])
 + return true;
 +
 + return false;
 +}
 +
 +#define __gen8_read(x) \
 +static u##x \
 +gen8_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 + bool __needs_put = reg  0x4  !is_gen8_shadowed(dev_priv, reg); \

This isn't right. Shadowed registers still need forcewake for reads.

 + REG_READ_HEADER(x); \
 + __needs_put = dev_priv-uncore.forcewake_count == 0; \

This looks a bit funky. I'm not sure I understood the issue correctly,
but it looks like just a failure to obey forcewake_count in gen8_write.
So I'd just fix gen8_write. Something like this:

 #define __gen8_write(x) \
 static void \
 gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
trace) { \
-   bool __needs_put = reg  0x4  !is_gen8_shadowed(dev_priv, reg); \
REG_WRITE_HEADER; \
-   if (__needs_put) { \
-   dev_priv-uncore.funcs.force_wake_get(dev_priv, \
-   FORCEWAKE_ALL); \
-   } \
-   __raw_i915_write##x(dev_priv, reg, val); \
-   if (__needs_put) { \
-   dev_priv-uncore.funcs.force_wake_put(dev_priv, \
-   FORCEWAKE_ALL); \
+   if (reg  0x4  !is_gen8_shadowed(dev_priv, reg)) { \
+   if (dev_priv-uncore.forcewake_count == 0)\
+   dev_priv-uncore.funcs.force_wake_get(dev_priv, \
+ FORCEWAKE_ALL); \
+   __raw_i915_write##x(dev_priv, reg, val); \
+   if (dev_priv-uncore.forcewake_count == 0) \
+   dev_priv-uncore.funcs.force_wake_put(dev_priv, \
+ FORCEWAKE_ALL); \
+   } else { \
+   __raw_i915_write##x(dev_priv, reg, val); \
} \
REG_WRITE_FOOTER; \
 }

I also noticed that gen6 doesn't increment/decrement the forcewake_count
here. We can follow that rule here too since we have the uncore lock
around the whole operation. I see the VLV code has the ++/--. I think we
should either add them ++/-- everywhere, or remove them from the VLV
code, just to make the code more uniform.

-- 
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 11/19] drm/i915: vlv: keep first level vblank IRQs masked

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 12:02:12AM +0200, Imre Deak wrote:
 This is a left-over from
 
 commit b7e634cc8dcd320123199a18bae0937b40dc28b8
 Author: Imre Deak imre.d...@intel.com
 Date:   Tue Feb 4 21:35:45 2014 +0200
 
 drm/i915: vlv: don't unmask IIR[DISPLAY_PIPE_A/B_VBLANK] interrupt
 
 where we stopped unmasking the vblank IRQs, but left them enabled in the
 IER register. Disable them in IER too.
 
 Signed-off-by: Imre Deak imre.d...@intel.com
 ---
  drivers/gpu/drm/i915/i915_irq.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index f68aee3..75dd0a8 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -3026,17 +3026,13 @@ static int valleyview_irq_postinstall(struct 
 drm_device *dev)
  
   enable_mask = I915_DISPLAY_PORT_INTERRUPT;
   enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
 - I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT |
 - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
 - I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
 + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
  
   /*
*Leave vblank interrupts masked initially.  enable/disable will
* toggle them based on usage.
*/

This comment is now stale.

 - dev_priv-irq_mask = (~enable_mask) |
 - I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT |
 - I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
 + dev_priv-irq_mask = ~enable_mask;
  
   I915_WRITE(PORT_HOTPLUG_EN, 0);
   POSTING_READ(PORT_HOTPLUG_EN);
 -- 
 1.8.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] build: Skip kms_plane and kms_sysfs_edid_timing on Android

2014-02-18 Thread Damien Lespiau
On Tue, Feb 18, 2014 at 04:58:58PM +, Santos, Joao wrote:
 Someone asked me to put the kms_sysfs_edid_timing for the same reason,
 I honestly didn't check it myself.  I think we should still add
 kms_plane to the skip_list because otherwise it blocks anyone who
 tries to build igt tests.

Yes, kms_plane should be skipped (until we have cairo in the build). Can
you please ask details about kms_sysfs_edid_timing?

 I'll send another patch if that's alright (?).

Yes, please.

-- 
Damien

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


Re: [Intel-gfx] [PATCH] Update intel_chipset macros

2014-02-18 Thread Santos, Joao
The idea behind this patch was indeed to standardize both libdrm's and igt's 
intel_chipset.h.
That's sounds like a (!very) sane idea to me. I really don't mind taking that 
on later but right now I need to get a patch tested out of the door. 

Joao.

-Original Message-
From: Lespiau, Damien 
Sent: Tuesday, February 18, 2014 2:27 PM
To: Santos, Joao
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] Update intel_chipset macros

On Tue, Feb 18, 2014 at 01:15:12PM +, joao.san...@intel.com wrote:
 From: Joao Santos joao.san...@intel.com
 
 Added in new macros to support new platforms.

What's the real purpose of this patch? you're adding a bunch of defines here 
without any user. I'd argue that we really want to move away from those 
defines, they are quite dreadful.

If the purpose is to make libdrm's and intel-gpu-tools' intel_chipset.h look 
alike, a worthy goal for sure, I'd vote for splitting the i-g-t specific 
defines into a separate file in i-g-t, waiting for someone fed-up enough with 
the current situation to create a more structured way to store per-platform 
feature flags/limits.

--
Damien

 
 Signed-off-by: Joao Santos joao.san...@intel.com
 ---
  intel/intel_chipset.h |   49 
 +++--
  1 file changed, 43 insertions(+), 6 deletions(-)
 
 diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h index 
 e5589be..f7771a7 100644
 --- a/intel/intel_chipset.h
 +++ b/intel/intel_chipset.h
 @@ -314,12 +314,49 @@
  
  #define IS_GEN8(devid)   IS_BROADWELL(devid)
  
 -#define IS_9XX(dev)  (IS_GEN3(dev) || \
 -  IS_GEN4(dev) || \
 -  IS_GEN5(dev) || \
 -  IS_GEN6(dev) || \
 -  IS_GEN7(dev) || \
 -  IS_GEN8(dev))
 +#define IS_965(devid)(IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_9XX(devid)(IS_GEN3(devid) || \
 +  IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_INTEL(devid)  (IS_GEN2(devid) || \
 +  IS_GEN3(devid) || \
 +  IS_GEN4(devid) || \
 +  IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_PCH_SPLIT(devid) (IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_IVYBRIDGE(devid) || IS_HASWELL(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_BLT_RING(devid)  (IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define HAS_BSD_RING(devid)  (IS_GEN5(devid) || \
 +  IS_GEN6(devid) || \
 +  IS_GEN7(devid) || \
 +  IS_GEN8(devid))
 +
 +#define IS_BROADWATER(devid) ((devid) == PCI_CHIP_I946_GZ || \
 +  (devid) == PCI_CHIP_I965_G_1 || \
 +  (devid) == PCI_CHIP_I965_Q || \
 +  (devid) == PCI_CHIP_I965_G)
 +
 +#define IS_CRESTLINE(devid)  ((devid) == PCI_CHIP_I965_GM || \
 +  (devid) == PCI_CHIP_I965_GME)
  
 +#define HAS_VEBOX_RING(devid)   (IS_HASWELL(devid))
  
  #endif /* _INTEL_CHIPSET_H */
 --
 1.7.9.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] [PATCH] drm/i915: Fix forcewake counts for gen8

2014-02-18 Thread Mika Kuoppala
Sometimes generic driver code gets forcewake explicitly by
gen6_gt_force_wake_get(), which check forcewake_count before accessing
hardware. However the register access with gen8_write function access
low level hw accessors directly, ignoring the forcewake_count. This
leads to nested forcewake get from hardware, in ring init and possibly
elsewhere, causing forcewake ack clear errors and/or hangs.

Fix this by checking the forcewake count also in gen8_write

v2: Read side doesn't care about shadowed registers,
Remove __needs_put funkiness from gen8_write. (Ville)
Improved commit message.

References: https://bugs.freedesktop.org/show_bug.cgi?id=74007
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Cc: Ben Widawsky benjamin.widaw...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
 drivers/gpu/drm/i915/intel_uncore.c |   19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index c628414..d1e9d63 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -634,16 +634,17 @@ static bool is_gen8_shadowed(struct drm_i915_private 
*dev_priv, u32 reg)
 #define __gen8_write(x) \
 static void \
 gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
trace) { \
-   bool __needs_put = reg  0x4  !is_gen8_shadowed(dev_priv, reg); \
REG_WRITE_HEADER; \
-   if (__needs_put) { \
-   dev_priv-uncore.funcs.force_wake_get(dev_priv, \
-   FORCEWAKE_ALL); \
-   } \
-   __raw_i915_write##x(dev_priv, reg, val); \
-   if (__needs_put) { \
-   dev_priv-uncore.funcs.force_wake_put(dev_priv, \
-   FORCEWAKE_ALL); \
+   if (reg  0x4  !is_gen8_shadowed(dev_priv, reg)) { \
+   if (dev_priv-uncore.forcewake_count == 0) \
+   dev_priv-uncore.funcs.force_wake_get(dev_priv, \
+ FORCEWAKE_ALL); \
+   __raw_i915_write##x(dev_priv, reg, val); \
+   if (dev_priv-uncore.forcewake_count == 0) \
+   dev_priv-uncore.funcs.force_wake_put(dev_priv, \
+ FORCEWAKE_ALL); \
+   } else { \
+   __raw_i915_write##x(dev_priv, reg, val); \
} \
REG_WRITE_FOOTER; \
 }
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH 16/19] drm/i915: sanity check power well sw state against hw state

2014-02-18 Thread Imre Deak
On Tue, 2014-02-18 at 18:55 +0200, Ville Syrjälä wrote:
 On Tue, Feb 18, 2014 at 12:02:17AM +0200, Imre Deak wrote:
  Suggested by Daniel.
  
  Signed-off-by: Imre Deak imre.d...@intel.com
  ---
   drivers/gpu/drm/i915/intel_pm.c | 33 ++---
   1 file changed, 30 insertions(+), 3 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index e81e7de..21ccf89 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -5338,6 +5338,24 @@ static void hsw_power_well_disable(struct 
  drm_i915_private *dev_priv,
  hsw_enable_package_c8(dev_priv);
   }
   
  +static void check_power_well_state(struct drm_i915_private *dev_priv,
  +  struct i915_power_well *power_well)
  +{
  +   bool enabled;
  +
  +   if (!power_well-ops-is_enabled)
  +   return;
  +
  +   enabled = power_well-ops-is_enabled(dev_priv, power_well);
  +
  +   if (enabled != (power_well-count  0 || !i915.disable_power_well)) {
 
 Doesn't i915.disable_power_well==true mean leave power wells always
 enabled? So I think the '!' needs to be removed.

No, i915.disable_power_well==true means disable power wells when the
refcount goes to 0. Perhaps not the best name/semantics for this kind of
option, the default for it should be 0 and mean normal operation, which
is to disable power wells when possible.

  +   DRM_ERROR(state mismatch for '%s' (hw state %d use-count %d 
  disable_power_well %d\n,
  + power_well-name, enabled, power_well-count,
  + i915.disable_power_well);
  +   WARN_ON(1);
  +   }
 
 For an error message + backtrace, you could just use WARN().

Ok.

  +}
  +
   void intel_display_power_get(struct drm_i915_private *dev_priv,
   enum intel_display_power_domain domain)
   {
  @@ -5349,9 +5367,14 @@ void intel_display_power_get(struct drm_i915_private 
  *dev_priv,
   
  mutex_lock(power_domains-lock);
   
  -   for_each_power_well(i, power_well, BIT(domain), power_domains)
  -   if (!power_well-count++  power_well-ops-enable)
  +   for_each_power_well(i, power_well, BIT(domain), power_domains) {
  +   if (!power_well-count++  power_well-ops-enable) {
  +   DRM_DEBUG_KMS(enabling %s\n, power_well-name);
  power_well-ops-enable(dev_priv, power_well);
  +   }
  +
  +   check_power_well_state(dev_priv, power_well);
  +   }
   
  power_domains-domain_use_count[domain]++;
   
  @@ -5376,8 +5399,12 @@ void intel_display_power_put(struct drm_i915_private 
  *dev_priv,
  WARN_ON(!power_well-count);
   
  if (!--power_well-count  power_well-ops-disable 
  -   i915.disable_power_well)
  +   i915.disable_power_well) {
  +   DRM_DEBUG_KMS(disabling %s\n, power_well-name);
  power_well-ops-disable(dev_priv, power_well);
  +   }
  +
  +   check_power_well_state(dev_priv, power_well);
  }
   
  mutex_unlock(power_domains-lock);
  -- 
  1.8.4
  
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 



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 16/19] drm/i915: sanity check power well sw state against hw state

2014-02-18 Thread Ville Syrjälä
On Tue, Feb 18, 2014 at 07:37:01PM +0200, Imre Deak wrote:
 On Tue, 2014-02-18 at 18:55 +0200, Ville Syrjälä wrote:
  On Tue, Feb 18, 2014 at 12:02:17AM +0200, Imre Deak wrote:
   Suggested by Daniel.
   
   Signed-off-by: Imre Deak imre.d...@intel.com
   ---
drivers/gpu/drm/i915/intel_pm.c | 33 ++---
1 file changed, 30 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index e81e7de..21ccf89 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -5338,6 +5338,24 @@ static void hsw_power_well_disable(struct 
   drm_i915_private *dev_priv,
 hsw_enable_package_c8(dev_priv);
}

   +static void check_power_well_state(struct drm_i915_private *dev_priv,
   +struct i915_power_well *power_well)
   +{
   + bool enabled;
   +
   + if (!power_well-ops-is_enabled)
   + return;
   +
   + enabled = power_well-ops-is_enabled(dev_priv, power_well);
   +
   + if (enabled != (power_well-count  0 || !i915.disable_power_well)) {
  
  Doesn't i915.disable_power_well==true mean leave power wells always
  enabled? So I think the '!' needs to be removed.
 
 No, i915.disable_power_well==true means disable power wells when the
 refcount goes to 0. Perhaps not the best name/semantics for this kind of
 option, the default for it should be 0 and mean normal operation, which
 is to disable power wells when possible.

Oh I had the impression it was the other way around, but you're right.
Seems I keep getting confused by this thing. It has happened before and
I'm guessing it will happen again after I've forgotten the details
again.

 
   + DRM_ERROR(state mismatch for '%s' (hw state %d use-count %d 
   disable_power_well %d\n,
   +   power_well-name, enabled, power_well-count,
   +   i915.disable_power_well);
   + WARN_ON(1);
   + }
  
  For an error message + backtrace, you could just use WARN().
 
 Ok.
 
   +}
   +
void intel_display_power_get(struct drm_i915_private *dev_priv,
  enum intel_display_power_domain domain)
{
   @@ -5349,9 +5367,14 @@ void intel_display_power_get(struct 
   drm_i915_private *dev_priv,

 mutex_lock(power_domains-lock);

   - for_each_power_well(i, power_well, BIT(domain), power_domains)
   - if (!power_well-count++  power_well-ops-enable)
   + for_each_power_well(i, power_well, BIT(domain), power_domains) {
   + if (!power_well-count++  power_well-ops-enable) {
   + DRM_DEBUG_KMS(enabling %s\n, power_well-name);
 power_well-ops-enable(dev_priv, power_well);
   + }
   +
   + check_power_well_state(dev_priv, power_well);
   + }

 power_domains-domain_use_count[domain]++;

   @@ -5376,8 +5399,12 @@ void intel_display_power_put(struct 
   drm_i915_private *dev_priv,
 WARN_ON(!power_well-count);

 if (!--power_well-count  power_well-ops-disable 
   - i915.disable_power_well)
   + i915.disable_power_well) {
   + DRM_DEBUG_KMS(disabling %s\n, power_well-name);
 power_well-ops-disable(dev_priv, power_well);
   + }
   +
   + check_power_well_state(dev_priv, power_well);
 }

 mutex_unlock(power_domains-lock);
   -- 
   1.8.4
   
   ___
   Intel-gfx mailing list
   Intel-gfx@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/intel-gfx
  
 



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


[Intel-gfx] [PATCH 07/13] drm/i915: Add register whitelist for DRM master

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

These are used to implement scanline waits in the X server.

v2: Use #defines instead of magic numbers

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 29 +
 drivers/gpu/drm/i915/i915_reg.h|  6 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 4347a30..353e5cf 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -281,6 +281,19 @@ static const u32 gen7_blt_regs[] = {
BCS_SWCTRL,
 };
 
+static const u32 ivb_master_regs[] = {
+   FORCEWAKE_MT,
+   DERRMR,
+   GEN7_PIPE_DE_LOAD_SL(PIPE_A),
+   GEN7_PIPE_DE_LOAD_SL(PIPE_B),
+   GEN7_PIPE_DE_LOAD_SL(PIPE_C),
+};
+
+static const u32 hsw_master_regs[] = {
+   FORCEWAKE_MT,
+   DERRMR,
+};
+
 #undef REG64
 
 static u32 gen7_render_get_cmd_length_mask(u32 cmd_header)
@@ -409,6 +422,14 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer 
*ring)
ring-reg_table = gen7_render_regs;
ring-reg_count = ARRAY_SIZE(gen7_render_regs);
 
+   if (IS_HASWELL(ring-dev)) {
+   ring-master_reg_table = hsw_master_regs;
+   ring-master_reg_count = ARRAY_SIZE(hsw_master_regs);
+   } else {
+   ring-master_reg_table = ivb_master_regs;
+   ring-master_reg_count = ARRAY_SIZE(ivb_master_regs);
+   }
+
ring-get_cmd_length_mask = gen7_render_get_cmd_length_mask;
break;
case VCS:
@@ -428,6 +449,14 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer 
*ring)
ring-reg_table = gen7_blt_regs;
ring-reg_count = ARRAY_SIZE(gen7_blt_regs);
 
+   if (IS_HASWELL(ring-dev)) {
+   ring-master_reg_table = hsw_master_regs;
+   ring-master_reg_count = ARRAY_SIZE(hsw_master_regs);
+   } else {
+   ring-master_reg_table = ivb_master_regs;
+   ring-master_reg_count = ARRAY_SIZE(ivb_master_regs);
+   }
+
ring-get_cmd_length_mask = gen7_blt_get_cmd_length_mask;
break;
case VECS:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1f2aeba..87523df 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -415,6 +415,12 @@
 /* There are the 4 64-bit counter registers, one for each stream output */
 #define GEN7_SO_NUM_PRIMS_WRITTEN(n) (0x5200 + (n) * 8)
 
+#define _GEN7_PIPEA_DE_LOAD_SL 0x70068
+#define _GEN7_PIPEB_DE_LOAD_SL 0x71068
+#define GEN7_PIPE_DE_LOAD_SL(pipe) _PIPE(pipe, \
+_GEN7_PIPEA_DE_LOAD_SL, \
+_GEN7_PIPEB_DE_LOAD_SL)
+
 /*
  * Reset registers
  */
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 09/13] drm/i915: Reject commands that explicitly generate interrupts

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

The driver leaves most interrupts masked during normal operation,
so there would have to be additional work to enable userspace to
safely request/receive an interrupt.

v2: trailing commas, rebased

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 22 --
 drivers/gpu/drm/i915/i915_reg.h|  1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 4f14a24..0351df1 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -115,7 +115,7 @@
  -- */
 static const struct drm_i915_cmd_descriptor common_cmds[] = {
CMD(  MI_NOOP,  SMI,F,  1,  S  ),
-   CMD(  MI_USER_INTERRUPT,SMI,F,  1,  S  ),
+   CMD(  MI_USER_INTERRUPT,SMI,F,  1,  R  ),
CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  M  ),
CMD(  MI_ARB_CHECK, SMI,F,  1,  S  ),
CMD(  MI_REPORT_HEAD,   SMI,F,  1,  S  ),
@@ -156,7 +156,7 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = 
{
CMD(  GFX_OP_PIPE_CONTROL(5),   S3D,   !F,  0xFF,   B,
  .bits = {{
.offset = 1,
-   .mask = PIPE_CONTROL_MMIO_WRITE,
+   .mask = (PIPE_CONTROL_MMIO_WRITE | PIPE_CONTROL_NOTIFY),
.expected = 0,
  }},  ),
 };
@@ -186,6 +186,12 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = 
{
CMD(  MI_ARB_ON_OFF,SMI,F,  1,  R  ),
CMD(  MI_STORE_DWORD_IMM,   SMI,   !F,  0xFF,   S  ),
CMD(  MI_UPDATE_GTT,SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_FLUSH_DW,  SMI,   !F,  0x3F,   B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_FLUSH_DW_NOTIFY,
+   .expected = 0,
+ }},  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
/*
 * MFX_WAIT doesn't fit the way we handle length for most commands.
@@ -199,6 +205,12 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
CMD(  MI_ARB_ON_OFF,SMI,F,  1,  R  ),
CMD(  MI_STORE_DWORD_IMM,   SMI,   !F,  0xFF,   S  ),
CMD(  MI_UPDATE_GTT,SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_FLUSH_DW,  SMI,   !F,  0x3F,   B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_FLUSH_DW_NOTIFY,
+   .expected = 0,
+ }},  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
 };
 
@@ -206,6 +218,12 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = {
CMD(  MI_DISPLAY_FLIP,  SMI,   !F,  0xFF,   R  ),
CMD(  MI_STORE_DWORD_IMM,   SMI,   !F,  0x3FF,  S  ),
CMD(  MI_UPDATE_GTT,SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_FLUSH_DW,  SMI,   !F,  0x3F,   B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_FLUSH_DW_NOTIFY,
+   .expected = 0,
+ }},  ),
CMD(  COLOR_BLT,S2D,   !F,  0x3F,   S  ),
CMD(  SRC_COPY_BLT, S2D,   !F,  0x3F,   S  ),
 };
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 11cca96..e6dd7e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -269,6 +269,7 @@
 #define   MI_FLUSH_DW_STORE_INDEX  (121)
 #define   MI_INVALIDATE_TLB(118)
 #define   MI_FLUSH_DW_OP_STOREDW   (114)
+#define   MI_FLUSH_DW_NOTIFY   (18)
 #define   MI_INVALIDATE_BSD(17)
 #define   MI_FLUSH_DW_USE_GTT  (12)
 #define   MI_FLUSH_DW_USE_PPGTT(02)
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 06/13] drm/i915: Add register whitelists for mesa

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

These registers are currently used by mesa for blitting,
transform feedback extensions, and performance monitoring
extensions.

v2: REG64 macro

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 45 ++
 drivers/gpu/drm/i915/i915_reg.h| 20 +++
 2 files changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index cf03ba6..4347a30 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -244,6 +244,45 @@ static const struct drm_i915_cmd_table hsw_blt_ring_cmds[] 
= {
{ hsw_blt_cmds, ARRAY_SIZE(hsw_blt_cmds) },
 };
 
+/*
+ * Register whitelists, sorted by increasing register offset.
+ *
+ * Some registers that userspace accesses are 64 bits. The register
+ * access commands only allow 32-bit accesses. Hence, we have to include
+ * entries for both halves of the 64-bit registers.
+ */
+
+/* Convenience macro for adding 64-bit registers */
+#define REG64(addr) (addr), (addr + sizeof(u32))
+
+static const u32 gen7_render_regs[] = {
+   REG64(HS_INVOCATION_COUNT),
+   REG64(DS_INVOCATION_COUNT),
+   REG64(IA_VERTICES_COUNT),
+   REG64(IA_PRIMITIVES_COUNT),
+   REG64(VS_INVOCATION_COUNT),
+   REG64(GS_INVOCATION_COUNT),
+   REG64(GS_PRIMITIVES_COUNT),
+   REG64(CL_INVOCATION_COUNT),
+   REG64(CL_PRIMITIVES_COUNT),
+   REG64(PS_INVOCATION_COUNT),
+   REG64(PS_DEPTH_COUNT),
+   REG64(GEN7_SO_NUM_PRIMS_WRITTEN(0)),
+   REG64(GEN7_SO_NUM_PRIMS_WRITTEN(1)),
+   REG64(GEN7_SO_NUM_PRIMS_WRITTEN(2)),
+   REG64(GEN7_SO_NUM_PRIMS_WRITTEN(3)),
+   GEN7_SO_WRITE_OFFSET(0),
+   GEN7_SO_WRITE_OFFSET(1),
+   GEN7_SO_WRITE_OFFSET(2),
+   GEN7_SO_WRITE_OFFSET(3),
+};
+
+static const u32 gen7_blt_regs[] = {
+   BCS_SWCTRL,
+};
+
+#undef REG64
+
 static u32 gen7_render_get_cmd_length_mask(u32 cmd_header)
 {
u32 client = (cmd_header  INSTR_CLIENT_MASK)  INSTR_CLIENT_SHIFT;
@@ -367,6 +406,9 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer 
*ring)
ring-cmd_table_count = ARRAY_SIZE(gen7_render_cmds);
}
 
+   ring-reg_table = gen7_render_regs;
+   ring-reg_count = ARRAY_SIZE(gen7_render_regs);
+
ring-get_cmd_length_mask = gen7_render_get_cmd_length_mask;
break;
case VCS:
@@ -383,6 +425,9 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer 
*ring)
ring-cmd_table_count = ARRAY_SIZE(gen7_blt_cmds);
}
 
+   ring-reg_table = gen7_blt_regs;
+   ring-reg_count = ARRAY_SIZE(gen7_blt_regs);
+
ring-get_cmd_length_mask = gen7_blt_get_cmd_length_mask;
break;
case VECS:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 23be06a..1f2aeba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -396,6 +396,26 @@
 #define SRC_COPY_BLT  ((0x229)|(0x4322))
 
 /*
+ * Registers used only by the command parser
+ */
+#define BCS_SWCTRL 0x22200
+
+#define HS_INVOCATION_COUNT 0x2300
+#define DS_INVOCATION_COUNT 0x2308
+#define IA_VERTICES_COUNT   0x2310
+#define IA_PRIMITIVES_COUNT 0x2318
+#define VS_INVOCATION_COUNT 0x2320
+#define GS_INVOCATION_COUNT 0x2328
+#define GS_PRIMITIVES_COUNT 0x2330
+#define CL_INVOCATION_COUNT 0x2338
+#define CL_PRIMITIVES_COUNT 0x2340
+#define PS_INVOCATION_COUNT 0x2348
+#define PS_DEPTH_COUNT  0x2350
+
+/* There are the 4 64-bit counter registers, one for each stream output */
+#define GEN7_SO_NUM_PRIMS_WRITTEN(n) (0x5200 + (n) * 8)
+
+/*
  * Reset registers
  */
 #define DEBUG_RESET_I830   0x6070
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 00/13] Gen7 batch buffer command parser

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Certain OpenGL features (e.g. transform feedback, performance monitoring)
require userspace code to submit batches containing commands such as
MI_LOAD_REGISTER_IMM to access various registers. Unfortunately, some
generations of the hardware will noop these commands in unsecure batches
(which includes all userspace batches submitted via i915) even though the
commands may be safe and represent the intended programming model of the device.

This series introduces a software command parser similar in operation to the
command parsing done in hardware for unsecure batches. However, the software
parser allows some operations that would be noop'd by hardware, if the parser
determines the operation is safe, and submits the batch as secure to prevent
hardware parsing. Currently the series implements this on IVB and HSW.

The series has one piece of prep work, one patch for the parser logic, and a
handful of patches to fill out the tables which drive the parser. There are
follow-up patches to libdrm and to i-g-t. The i-g-t tests are basic and do not
test all of the commands used by the parser on the assumption that I'm likely
to make the same mistakes in both the parser and the test.

I've previously run the i-g-t gem_* tests, the piglit quick tests, and generally
used Ubuntu 13.10 IVB and HSW systems with the parser running. Aside from a
failure described below, I did not see any regressions.

At this point there are a couple of required/potential improvements.

1) Chained batches. The parser currently allows MI_BATCH_BUFFER_START commands
   in userspace batches without parsing them. The media driver uses chained
   batches, so a solution is required. I'm still working through the
   requirements but don't want to continue delaying the review process for what
   I have so far.
2) Command buffer copy. To avoid CPU modifications to buffers after parsing, and
   to avoid GPU modifications to buffers via EUs or commands in the batch, we
   should copy the userspace batch buffer to memory that userspace does not
   have access to, map it into GGTT, and execute that batch buffer. I have a
   sense of how to do this for 1st-level batches, but it may need changes to
   tie in with the chained batch parsing, so I've again held off.
3) Coherency. I've previously found a coherency issue on VLV when reading the
   batch buffer from the CPU during execbuffer2. Userspace writes the batch via
   pwrite fast path before calling execbuffer2. The parser reads stale data.
   This works fine on IVB and HSW, so I believe it's an LLC vs. non-LLC issue.
   It's possible that the shmem pread refactoring fixes this, I just have not
   been able to retest due to lack of a VLV system.

v2:
- Significantly reorder series
- Scan secure batches (i.e. I915_EXEC_SECURE)
- Check that parser tables are sorted during init
- Fixed gem_cpu_reloc regression
- HAS_CMD_PARSER - CMD_PARSER_VERSION getparam
- Additional tests

v3:
- Don't actually send batches as secure yet
- Improved documentation and commenting
- Many other small cleanups throughout

Brad Volkin (13):
  drm/i915: Refactor shmem pread setup
  drm/i915: Implement command buffer parsing logic
  drm/i915: Initial command parser table definitions
  drm/i915: Reject privileged commands
  drm/i915: Allow some privileged commands from master
  drm/i915: Add register whitelists for mesa
  drm/i915: Add register whitelist for DRM master
  drm/i915: Enable register whitelist checks
  drm/i915: Reject commands that explicitly generate interrupts
  drm/i915: Enable PPGTT command parser checks
  drm/i915: Reject commands that would store to global HWS page
  drm/i915: Add a CMD_PARSER_VERSION getparam
  drm/i915: Enable command parsing by default

 drivers/gpu/drm/i915/Makefile  |   1 +
 drivers/gpu/drm/i915/i915_cmd_parser.c | 918 +
 drivers/gpu/drm/i915/i915_dma.c|   3 +
 drivers/gpu/drm/i915/i915_drv.h| 103 
 drivers/gpu/drm/i915/i915_gem.c|  51 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  18 +
 drivers/gpu/drm/i915/i915_params.c |   5 +
 drivers/gpu/drm/i915/i915_reg.h|  96 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c|   2 +
 drivers/gpu/drm/i915/intel_ringbuffer.h|  32 +
 include/uapi/drm/i915_drm.h|   1 +
 11 files changed, 1216 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_cmd_parser.c

-- 
1.8.3.2

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


[Intel-gfx] [PATCH 13/13] drm/i915: Enable command parsing by default

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

v2: rebased

OTC-Tracker: AXIA-4631
Change-Id: I6747457e1fe7494bd42787af51198fcba398ad78
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index aba0b9b..9e394bc 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -48,7 +48,7 @@ struct i915_params i915 __read_mostly = {
.reset = true,
.invert_brightness = 0,
.disable_display = 0,
-   .enable_cmd_parser = 0,
+   .enable_cmd_parser = 1,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -161,4 +161,4 @@ MODULE_PARM_DESC(disable_display, Disable display 
(default: false));
 
 module_param_named(enable_cmd_parser, i915.enable_cmd_parser, int, 0600);
 MODULE_PARM_DESC(enable_cmd_parser,
-   Enable command parsing (1=enabled, 0=disabled [default]));
+   Enable command parsing (1=enabled [default], 0=disabled));
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 3/6] tests/gem_exec_parse: Add tests for register whitelist

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/gem_exec_parse.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index ebf7116..48fde25 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -141,6 +141,7 @@ int fd;
 
 #define MI_ARB_ON_OFF (0x8  23)
 #define MI_DISPLAY_FLIP ((0x14  23) | 1)
+#define MI_LOAD_REGISTER_IMM ((0x22  23) | 1)
 
 #define GFX_OP_PIPE_CONTROL((0x329)|(0x327)|(0x224)|2)
 #define   PIPE_CONTROL_QW_WRITE(114)
@@ -213,6 +214,31 @@ igt_main
  -EINVAL));
}
 
+   igt_subtest(registers) {
+   uint32_t lri_bad[] = {
+   MI_LOAD_REGISTER_IMM,
+   0, // disallowed register address
+   0x1200,
+   MI_BATCH_BUFFER_END,
+   };
+   uint32_t lri_ok[] = {
+   MI_LOAD_REGISTER_IMM,
+   0x5280, // allowed register address (SO_WRITE_OFFSET[0])
+   0x1,
+   MI_BATCH_BUFFER_END,
+   };
+   igt_assert(
+  exec_batch(fd, handle,
+ lri_bad, sizeof(lri_bad),
+ I915_EXEC_RENDER,
+ -EINVAL));
+   igt_assert(
+  exec_batch(fd, handle,
+ lri_ok, sizeof(lri_ok),
+ I915_EXEC_RENDER,
+ 0));
+   }
+
igt_fixture {
gem_close(fd, handle);
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 05/13] drm/i915: Allow some privileged commands from master

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

The Intel DDX uses these to implement scanline waits in the X server.

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 90bbb6d..cf03ba6 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -116,7 +116,7 @@
 static const struct drm_i915_cmd_descriptor common_cmds[] = {
CMD(  MI_NOOP,  SMI,F,  1,  S  ),
CMD(  MI_USER_INTERRUPT,SMI,F,  1,  S  ),
-   CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  R  ),
+   CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  M  ),
CMD(  MI_ARB_CHECK, SMI,F,  1,  S  ),
CMD(  MI_REPORT_HEAD,   SMI,F,  1,  S  ),
CMD(  MI_SUSPEND_FLUSH, SMI,F,  1,  S  ),
@@ -151,7 +151,7 @@ static const struct drm_i915_cmd_descriptor 
hsw_render_cmds[] = {
CMD(  MI_RS_CONTROL,SMI,F,  1,  S  ),
CMD(  MI_URB_ATOMIC_ALLOC,  SMI,F,  1,  S  ),
CMD(  MI_RS_CONTEXT,SMI,F,  1,  S  ),
-   CMD(  MI_LOAD_SCAN_LINES_INCL,  SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_LOAD_SCAN_LINES_INCL,  SMI,   !F,  0x3F,   M  ),
CMD(  MI_LOAD_SCAN_LINES_EXCL,  SMI,   !F,  0x3F,   R  ),
CMD(  MI_LOAD_REGISTER_REG, SMI,   !F,  0xFF,   R  ),
CMD(  MI_RS_STORE_DATA_IMM, SMI,   !F,  0xFF,   S  ),
@@ -196,7 +196,7 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = {
 };
 
 static const struct drm_i915_cmd_descriptor hsw_blt_cmds[] = {
-   CMD(  MI_LOAD_SCAN_LINES_INCL,  SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_LOAD_SCAN_LINES_INCL,  SMI,   !F,  0x3F,   M  ),
CMD(  MI_LOAD_SCAN_LINES_EXCL,  SMI,   !F,  0x3F,   R  ),
 };
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 6/6] tests/gem_exec_parse: Test a command crossing a page boundary

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

This is a speculative test in that it's not particularly relevant
today, but is important if we switch the parser implementation to
use kmap_atomic instead of vmap.

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/gem_exec_parse.c | 68 ++
 1 file changed, 68 insertions(+)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 004c3bf..455bfbf 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -136,6 +136,60 @@ static int exec_batch(int fd, uint32_t cmd_bo, uint32_t 
*cmds,
return 1;
 }
 
+static int exec_split_batch(int fd, uint32_t *cmds,
+   int size, int ring, int expected_ret)
+{
+   struct drm_i915_gem_execbuffer2 execbuf;
+   struct drm_i915_gem_exec_object2 objs[1];
+   uint32_t cmd_bo;
+   uint32_t noop[1024] = { 0 };
+   int ret;
+
+   // Allocate and fill a 2-page batch with noops
+   cmd_bo = gem_create(fd, 4096 * 2);
+   gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
+   gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
+
+   // Write the provided commands such that the first dword
+   // of the command buffer is the last dword of the first
+   // page (i.e. the command is split across the two pages).
+   gem_write(fd, cmd_bo, 4096-sizeof(uint32_t), cmds, size);
+
+   objs[0].handle = cmd_bo;
+   objs[0].relocation_count = 0;
+   objs[0].relocs_ptr = 0;
+   objs[0].alignment = 0;
+   objs[0].offset = 0;
+   objs[0].flags = 0;
+   objs[0].rsvd1 = 0;
+   objs[0].rsvd2 = 0;
+
+   execbuf.buffers_ptr = (uintptr_t)objs;
+   execbuf.buffer_count = 1;
+   execbuf.batch_start_offset = 0;
+   execbuf.batch_len = size;
+   execbuf.cliprects_ptr = 0;
+   execbuf.num_cliprects = 0;
+   execbuf.DR1 = 0;
+   execbuf.DR4 = 0;
+   execbuf.flags = ring;
+   i915_execbuffer2_set_context_id(execbuf, 0);
+   execbuf.rsvd2 = 0;
+
+   ret = drmIoctl(fd,
+  DRM_IOCTL_I915_GEM_EXECBUFFER2,
+  execbuf);
+   if (ret == 0)
+   igt_assert(expected_ret == 0);
+   else
+   igt_assert(-errno == expected_ret);
+
+   gem_sync(fd, cmd_bo);
+   gem_close(fd, cmd_bo);
+
+   return 1;
+}
+
 uint32_t handle;
 int fd;
 
@@ -266,6 +320,20 @@ igt_main
  -EINVAL));
}
 
+   igt_subtest(cmd-crossing-page) {
+   uint32_t lri_ok[] = {
+   MI_LOAD_REGISTER_IMM,
+   0x5280, // allowed register address (SO_WRITE_OFFSET[0])
+   0x1,
+   MI_BATCH_BUFFER_END,
+   };
+   igt_assert(
+  exec_split_batch(fd,
+   lri_ok, sizeof(lri_ok),
+   I915_EXEC_RENDER,
+   0));
+   }
+
igt_fixture {
gem_close(fd, handle);
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 2/6] tests/gem_exec_parse: Add tests for rejected commands

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/gem_exec_parse.c | 81 ++
 1 file changed, 81 insertions(+)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index c71e478..ebf7116 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -93,9 +93,55 @@ static int exec_batch_patched(int fd, uint32_t cmd_bo, 
uint32_t *cmds,
return 1;
 }
 
+static int exec_batch(int fd, uint32_t cmd_bo, uint32_t *cmds,
+ int size, int ring, int expected_ret)
+{
+   struct drm_i915_gem_execbuffer2 execbuf;
+   struct drm_i915_gem_exec_object2 objs[1];
+   int ret;
+
+   gem_write(fd, cmd_bo, 0, cmds, size);
+
+   objs[0].handle = cmd_bo;
+   objs[0].relocation_count = 0;
+   objs[0].relocs_ptr = 0;
+   objs[0].alignment = 0;
+   objs[0].offset = 0;
+   objs[0].flags = 0;
+   objs[0].rsvd1 = 0;
+   objs[0].rsvd2 = 0;
+
+   execbuf.buffers_ptr = (uintptr_t)objs;
+   execbuf.buffer_count = 1;
+   execbuf.batch_start_offset = 0;
+   execbuf.batch_len = size;
+   execbuf.cliprects_ptr = 0;
+   execbuf.num_cliprects = 0;
+   execbuf.DR1 = 0;
+   execbuf.DR4 = 0;
+   execbuf.flags = ring;
+   i915_execbuffer2_set_context_id(execbuf, 0);
+   execbuf.rsvd2 = 0;
+
+   ret = drmIoctl(fd,
+  DRM_IOCTL_I915_GEM_EXECBUFFER2,
+  execbuf);
+   if (ret == 0)
+   igt_assert(expected_ret == 0);
+   else
+   igt_assert(-errno == expected_ret);
+
+   gem_sync(fd, cmd_bo);
+
+   return 1;
+}
+
 uint32_t handle;
 int fd;
 
+#define MI_ARB_ON_OFF (0x8  23)
+#define MI_DISPLAY_FLIP ((0x14  23) | 1)
+
 #define GFX_OP_PIPE_CONTROL((0x329)|(0x327)|(0x224)|2)
 #define   PIPE_CONTROL_QW_WRITE(114)
 
@@ -132,6 +178,41 @@ igt_main
   0x1200));
}
 
+   igt_subtest(basic-rejected) {
+   uint32_t arb_on_off[] = {
+   MI_ARB_ON_OFF,
+   MI_BATCH_BUFFER_END,
+   };
+   uint32_t display_flip[] = {
+   MI_DISPLAY_FLIP,
+   0, 0, 0,
+   MI_BATCH_BUFFER_END,
+   0
+   };
+   igt_assert(
+  exec_batch(fd, handle,
+ arb_on_off, sizeof(arb_on_off),
+ I915_EXEC_RENDER,
+ -EINVAL));
+   igt_assert(
+  exec_batch(fd, handle,
+ arb_on_off, sizeof(arb_on_off),
+ I915_EXEC_BSD,
+ -EINVAL));
+   if (gem_has_vebox(fd)) {
+   igt_assert(
+  exec_batch(fd, handle,
+ arb_on_off, sizeof(arb_on_off),
+ I915_EXEC_VEBOX,
+ -EINVAL));
+   }
+   igt_assert(
+  exec_batch(fd, handle,
+ display_flip, sizeof(display_flip),
+ I915_EXEC_BLT,
+ -EINVAL));
+   }
+
igt_fixture {
gem_close(fd, handle);
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 02/13] drm/i915: Implement command buffer parsing logic

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

The command parser scans batch buffers submitted via execbuffer ioctls before
the driver submits them to hardware. At a high level, it looks for several
things:

1) Commands which are explicitly defined as privileged or which should only be
   used by the kernel driver. The parser generally rejects such commands, with
   the provision that it may allow some from the drm master process.
2) Commands which access registers. To support correct/enhanced userspace
   functionality, particularly certain OpenGL extensions, the parser provides a
   whitelist of registers which userspace may safely access (for both normal and
   drm master processes).
3) Commands which access privileged memory (i.e. GGTT, HWS page, etc). The
   parser always rejects such commands.

See the overview comment in the source for more details.

This patch only implements the logic. Subsequent patches will build the tables
that drive the parser.

v2: Don't set the secure bit if the parser succeeds
Fail harder during init
Makefile cleanup
Kerneldoc cleanup
Clarify module param description
Convert ints to bools in a few places
Move client/subclient defs to i915_reg.h
Remove the bits_count field

OTC-Tracker: AXIA-4631
Change-Id: I50b98c71c6655893291c78a2d1b8954577b37a30
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/Makefile  |   1 +
 drivers/gpu/drm/i915/i915_cmd_parser.c | 485 +
 drivers/gpu/drm/i915/i915_drv.h|  93 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  18 ++
 drivers/gpu/drm/i915/i915_params.c |   5 +
 drivers/gpu/drm/i915/i915_reg.h|  12 +
 drivers/gpu/drm/i915/intel_ringbuffer.c|   2 +
 drivers/gpu/drm/i915/intel_ringbuffer.h|  32 ++
 8 files changed, 648 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_cmd_parser.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 4850494..3569122 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
  i915_gem_gtt.o \
  i915_gem_stolen.o \
  i915_gem_tiling.o \
+ i915_cmd_parser.o \
  i915_params.o \
  i915_sysfs.o \
  i915_trace_points.o \
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
new file mode 100644
index 000..7a5756e
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -0,0 +1,485 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Brad Volkin bradley.d.vol...@intel.com
+ *
+ */
+
+#include i915_drv.h
+
+/**
+ * DOC: i915 batch buffer command parser
+ *
+ * Motivation:
+ * Certain OpenGL features (e.g. transform feedback, performance monitoring)
+ * require userspace code to submit batches containing commands such as
+ * MI_LOAD_REGISTER_IMM to access various registers. Unfortunately, some
+ * generations of the hardware will noop these commands in unsecure batches
+ * (which includes all userspace batches submitted via i915) even though the
+ * commands may be safe and represent the intended programming model of the
+ * device.
+ *
+ * The software command parser is similar in operation to the command parsing
+ * done in hardware for unsecure batches. However, the software parser allows
+ * some operations that would be noop'd by hardware, if the parser determines
+ * the operation is safe, and submits the batch as secure to prevent hardware
+ * parsing.
+ *
+ * Threats:
+ * At a high level, the hardware (and software) checks attempt to prevent
+ * granting userspace undue privileges. There are three categories of 
privilege.
+ *
+ * First, commands which are explicitly defined as privileged or which should
+ * only be used by the kernel driver. The 

[Intel-gfx] [PATCH 1/6] tests: Add a test for the command parser

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Start with a simple testcase that should pass.

v2: Switch to I915_PARAM_CMD_PARSER_VERSION

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/gem_exec_parse.c | 140 +
 3 files changed, 142 insertions(+)
 create mode 100644 tests/gem_exec_parse.c

diff --git a/tests/.gitignore b/tests/.gitignore
index cb548a8..8b0b790 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -35,6 +35,7 @@ gem_exec_blt
 gem_exec_faulting_reloc
 gem_exec_lut_handle
 gem_exec_nop
+gem_exec_parse
 gem_fd_exhaustion
 gem_fenced_exec_thrash
 gem_fence_thrash
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index afb2582..2475f7e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -29,6 +29,7 @@ TESTS_progs_M = \
gem_exec_bad_domains \
gem_exec_faulting_reloc \
gem_exec_nop \
+   gem_exec_parse \
gem_fenced_exec_thrash \
gem_fence_thrash \
gem_flink \
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
new file mode 100644
index 000..c71e478
--- /dev/null
+++ b/tests/gem_exec_parse.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+
+#ifndef I915_PARAM_CMD_PARSER_VERSION
+#define I915_PARAM_CMD_PARSER_VERSION   28
+#endif
+
+static int exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
+ int size, int patch_offset, uint64_t 
expected_value)
+{
+   struct drm_i915_gem_execbuffer2 execbuf;
+   struct drm_i915_gem_exec_object2 objs[2];
+   struct drm_i915_gem_relocation_entry reloc[1];
+
+   uint32_t target_bo = gem_create(fd, 4096);
+   uint64_t actual_value = 0;
+
+   gem_write(fd, cmd_bo, 0, cmds, size);
+
+   reloc[0].offset = patch_offset;
+   reloc[0].delta = 0;
+   reloc[0].target_handle = target_bo;
+   reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
+   reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
+   reloc[0].presumed_offset = 0;
+
+   objs[0].handle = target_bo;
+   objs[0].relocation_count = 0;
+   objs[0].relocs_ptr = 0;
+   objs[0].alignment = 0;
+   objs[0].offset = 0;
+   objs[0].flags = 0;
+   objs[0].rsvd1 = 0;
+   objs[0].rsvd2 = 0;
+
+   objs[1].handle = cmd_bo;
+   objs[1].relocation_count = 1;
+   objs[1].relocs_ptr = (uintptr_t)reloc;
+   objs[1].alignment = 0;
+   objs[1].offset = 0;
+   objs[1].flags = 0;
+   objs[1].rsvd1 = 0;
+   objs[1].rsvd2 = 0;
+
+   execbuf.buffers_ptr = (uintptr_t)objs;
+   execbuf.buffer_count = 2;
+   execbuf.batch_start_offset = 0;
+   execbuf.batch_len = size;
+   execbuf.cliprects_ptr = 0;
+   execbuf.num_cliprects = 0;
+   execbuf.DR1 = 0;
+   execbuf.DR4 = 0;
+   execbuf.flags = I915_EXEC_RENDER;
+   i915_execbuffer2_set_context_id(execbuf, 0);
+   execbuf.rsvd2 = 0;
+
+   gem_execbuf(fd, execbuf);
+   gem_sync(fd, cmd_bo);
+
+   gem_read(fd,target_bo, 0, actual_value, sizeof(actual_value));
+   igt_assert(expected_value == actual_value);
+
+   gem_close(fd, target_bo);
+
+   return 1;
+}
+
+uint32_t handle;
+int fd;
+
+#define GFX_OP_PIPE_CONTROL((0x329)|(0x327)|(0x224)|2)
+#define   PIPE_CONTROL_QW_WRITE(114)
+
+igt_main
+{
+   igt_fixture {
+   int parser_version = 0;
+drm_i915_getparam_t gp;
+   int rc;
+
+   fd = drm_open_any();
+
+   gp.param = I915_PARAM_CMD_PARSER_VERSION;
+   gp.value = parser_version;
+   rc = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, 

[Intel-gfx] [PATCH 4/6] tests/gem_exec_parse: Add tests for bitmask checks

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/gem_exec_parse.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 48fde25..9e90408 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -145,6 +145,7 @@ int fd;
 
 #define GFX_OP_PIPE_CONTROL((0x329)|(0x327)|(0x224)|2)
 #define   PIPE_CONTROL_QW_WRITE(114)
+#define   PIPE_CONTROL_LRI_POST_OP (123)
 
 igt_main
 {
@@ -239,6 +240,23 @@ igt_main
  0));
}
 
+   igt_subtest(bitmasks) {
+   uint32_t pc[] = {
+   GFX_OP_PIPE_CONTROL,
+   (PIPE_CONTROL_QW_WRITE |
+PIPE_CONTROL_LRI_POST_OP),
+   0, // To be patched
+   0x1200,
+   0,
+   MI_BATCH_BUFFER_END,
+   };
+   igt_assert(
+  exec_batch(fd, handle,
+ pc, sizeof(pc),
+ I915_EXEC_RENDER,
+ -EINVAL));
+   }
+
igt_fixture {
gem_close(fd, handle);
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 03/13] drm/i915: Initial command parser table definitions

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Add command tables defining irregular length commands for each ring.
This requires a few new command opcode definitions.

v2: Whitespace adjustment in command definitions, sparse fix for !F

OTC-Tracker: AXIA-4631
Change-Id: I064bceb457e15f46928058352afe76d918c58ef5
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 157 +
 drivers/gpu/drm/i915/i915_reg.h|  46 ++
 2 files changed, 203 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 7a5756e..12241e8 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -86,6 +86,148 @@
  * general bitmasking mechanism.
  */
 
+#define STD_MI_OPCODE_MASK  0xFF80
+#define STD_3D_OPCODE_MASK  0x
+#define STD_2D_OPCODE_MASK  0xFFC0
+#define STD_MFX_OPCODE_MASK 0x
+
+#define CMD(op, opm, f, lm, fl, ...)   \
+   {   \
+   .flags = (fl) | ((f) ? CMD_DESC_FIXED : 0), \
+   .cmd = { (op), (opm) }, \
+   .length = { (lm) }, \
+   __VA_ARGS__ \
+   }
+
+/* Convenience macros to compress the tables */
+#define SMI STD_MI_OPCODE_MASK
+#define S3D STD_3D_OPCODE_MASK
+#define S2D STD_2D_OPCODE_MASK
+#define SMFX STD_MFX_OPCODE_MASK
+#define F true
+#define S CMD_DESC_SKIP
+#define R CMD_DESC_REJECT
+#define W CMD_DESC_REGISTER
+#define B CMD_DESC_BITMASK
+#define M CMD_DESC_MASTER
+
+/*Command  Mask   Fixed Len   Action
+ -- */
+static const struct drm_i915_cmd_descriptor common_cmds[] = {
+   CMD(  MI_NOOP,  SMI,F,  1,  S  ),
+   CMD(  MI_USER_INTERRUPT,SMI,F,  1,  S  ),
+   CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  S  ),
+   CMD(  MI_ARB_CHECK, SMI,F,  1,  S  ),
+   CMD(  MI_REPORT_HEAD,   SMI,F,  1,  S  ),
+   CMD(  MI_SUSPEND_FLUSH, SMI,F,  1,  S  ),
+   CMD(  MI_SEMAPHORE_MBOX,SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_BATCH_BUFFER_START,SMI,   !F,  0xFF,   S  ),
+};
+
+static const struct drm_i915_cmd_descriptor render_cmds[] = {
+   CMD(  MI_FLUSH, SMI,F,  1,  S  ),
+   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  S  ),
+   CMD(  MI_PREDICATE, SMI,F,  1,  S  ),
+   CMD(  MI_TOPOLOGY_FILTER,   SMI,F,  1,  S  ),
+   CMD(  MI_DISPLAY_FLIP,  SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_SET_CONTEXT,   SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_URB_CLEAR, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_UPDATE_GTT,SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_CLFLUSH,   SMI,   !F,  0x3FF,  S  ),
+   CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
+   CMD(  GFX_OP_3DSTATE_VF_STATISTICS, S3D,F,  1,  S  ),
+   CMD(  PIPELINE_SELECT,  S3D,F,  1,  S  ),
+   CMD(  GPGPU_OBJECT, S3D,   !F,  0xFF,   S  ),
+   CMD(  GPGPU_WALKER, S3D,   !F,  0xFF,   S  ),
+   CMD(  GFX_OP_3DSTATE_SO_DECL_LIST,  S3D,   !F,  0x1FF,  S  ),
+};
+
+static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = {
+   CMD(  MI_SET_PREDICATE, SMI,F,  1,  S  ),
+   CMD(  MI_RS_CONTROL,SMI,F,  1,  S  ),
+   CMD(  MI_URB_ATOMIC_ALLOC,  SMI,F,  1,  S  ),
+   CMD(  MI_RS_CONTEXT,SMI,F,  1,  S  ),
+   CMD(  MI_LOAD_REGISTER_REG, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_RS_STORE_DATA_IMM, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_LOAD_URB_MEM,  SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_STORE_URB_MEM, SMI,   !F,  0xFF,   S  ),
+   CMD(  GFX_OP_3DSTATE_DX9_CONSTANTF_VS,  S3D,   !F,  0x7FF,  S  ),
+   CMD(  GFX_OP_3DSTATE_DX9_CONSTANTF_PS,  S3D,   !F,  0x7FF,  S  ),
+
+   CMD(  GFX_OP_3DSTATE_BINDING_TABLE_EDIT_VS,  S3D,   !F,  0x1FF,  S  ),
+   CMD(  GFX_OP_3DSTATE_BINDING_TABLE_EDIT_GS,  S3D,   !F,  0x1FF,  S  ),
+   CMD(  

[Intel-gfx] [PATCH] intel: Merge i915_drm.h with cmd parser define

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 include/drm/i915_drm.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 2f4eb8c..ba863c4 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -27,7 +27,7 @@
 #ifndef _I915_DRM_H_
 #define _I915_DRM_H_
 
-#include drm.h
+#include drm/drm.h
 
 /* Please note that modifications to all structs defined here are
  * subject to backwards-compatibility constraints.
@@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_NO_RELOC25
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT   27
+#define I915_PARAM_CMD_PARSER_VERSION   28
 
 typedef struct drm_i915_getparam {
int param;
@@ -721,7 +722,7 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_IS_PINNED(110)
 
-/** Provide a hint to the kernel that the command stream and auxilliary
+/** Provide a hint to the kernel that the command stream and auxiliary
  * state buffers already holds the correct presumed addresses and so the
  * relocation process may be skipped if no buffers need to be moved in
  * preparation for the execbuffer.
-- 
1.8.5.2

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


[Intel-gfx] [PATCH 10/13] drm/i915: Enable PPGTT command parser checks

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Various commands that access memory have a bit to determine whether
the graphics address specified in the command should use the GGTT or
PPGTT for translation. These checks ensure that the bit indicates
PPGTT translation.

Most of these checks use the existing bit-checking infrastructure.
The PIPE_CONTROL and MI_FLUSH_DW commands, however, are multi-function
commands. The GGTT/PPGTT bit is only relevant for certain uses of the
command. As such, this change also extends the bit-checking code to
include a condition mask and offset. If the condition mask is non-zero
then the parser only performs the bit check when the bits specified by
the condition mask/offset are also non-zero.

NOTE: At this point in the series PPGTT must be enabled for the parser
to work correctly. If it's not enabled, userspace will not be setting
the PPGTT bits the way the parser requires. VLV is the only platform
where this is a problem, so at this point, we disable parsing for VLV.

v2: whitespace and trailing commas fixes, rebased

OTC-Tracker: AXIA-4631
Change-Id: I3f4c76b6734f1956ec47e698230f97d0998ff92b
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 128 ++---
 drivers/gpu/drm/i915/i915_drv.h|   6 ++
 drivers/gpu/drm/i915/i915_reg.h|   6 ++
 3 files changed, 129 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 0351df1..1528549 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -124,10 +124,20 @@ static const struct drm_i915_cmd_descriptor common_cmds[] 
= {
CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   R  ),
CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   W,
  .reg = { .offset = 1, .mask = 0x007C }   ),
-   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   W,
- .reg = { .offset = 1, .mask = 0x007C }   ),
-   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   W,
- .reg = { .offset = 1, .mask = 0x007C }   ),
+   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   W | B,
+ .reg = { .offset = 1, .mask = 0x007C },
+ .bits = {{
+   .offset = 0,
+   .mask = MI_GLOBAL_GTT,
+   .expected = 0,
+ }},  ),
+   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   W | B,
+ .reg = { .offset = 1, .mask = 0x007C },
+ .bits = {{
+   .offset = 0,
+   .mask = MI_GLOBAL_GTT,
+   .expected = 0,
+ }},  ),
CMD(  MI_BATCH_BUFFER_START,SMI,   !F,  0xFF,   S  ),
 };
 
@@ -139,9 +149,31 @@ static const struct drm_i915_cmd_descriptor render_cmds[] 
= {
CMD(  MI_DISPLAY_FLIP,  SMI,   !F,  0xFF,   R  ),
CMD(  MI_SET_CONTEXT,   SMI,   !F,  0xFF,   R  ),
CMD(  MI_URB_CLEAR, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_STORE_DWORD_IMM,   SMI,   !F,  0x3F,   B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_GLOBAL_GTT,
+   .expected = 0,
+ }},  ),
CMD(  MI_UPDATE_GTT,SMI,   !F,  0xFF,   R  ),
-   CMD(  MI_CLFLUSH,   SMI,   !F,  0x3FF,  S  ),
-   CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_CLFLUSH,   SMI,   !F,  0x3FF,  B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_GLOBAL_GTT,
+   .expected = 0,
+ }},  ),
+   CMD(  MI_REPORT_PERF_COUNT, SMI,   !F,  0x3F,   B,
+ .bits = {{
+   .offset = 1,
+   .mask = MI_REPORT_PERF_COUNT_GGTT,
+   .expected = 0,
+ }},  ),
+   CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   B,
+ .bits = {{
+   .offset = 0,
+   .mask = MI_GLOBAL_GTT,
+   .expected = 0,
+ }},  ),
CMD(  GFX_OP_3DSTATE_VF_STATISTICS, S3D,F,  1,  S  ),
CMD(  PIPELINE_SELECT,  S3D,F,  1,  S  ),
CMD(  MEDIA_VFE_STATE,  S3D,   !F,  0x, B,
@@ -158,6 +190,13 @@ static const 

[Intel-gfx] [PATCH 01/13] drm/i915: Refactor shmem pread setup

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

The command parser is going to need the same synchronization and
setup logic, so factor it out for reuse.

v2: Add a check that the object is backed by shmem

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_gem.c | 51 ++---
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c64831..582035b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2097,6 +2097,9 @@ void i915_gem_release_all_mmaps(struct drm_i915_private 
*dev_priv);
 void i915_gem_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_lastclose(struct drm_device *dev);
 
+int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
+   int *needs_clflush);
+
 int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
 static inline struct page *i915_gem_object_get_page(struct drm_i915_gem_object 
*obj, int n)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3618bb0..83990cb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -326,6 +326,42 @@ __copy_from_user_swizzled(char *gpu_vaddr, int gpu_offset,
return 0;
 }
 
+/*
+ * Pins the specified object's pages and synchronizes the object with
+ * GPU accesses. Sets needs_clflush to non-zero if the caller should
+ * flush the object from the CPU cache.
+ */
+int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
+   int *needs_clflush)
+{
+   int ret;
+
+   *needs_clflush = 0;
+
+   if (!obj-base.filp)
+   return -EINVAL;
+
+   if (!(obj-base.read_domains  I915_GEM_DOMAIN_CPU)) {
+   /* If we're not in the cpu read domain, set ourself into the gtt
+* read domain and manually flush cachelines (if required). This
+* optimizes for the case when the gpu will dirty the data
+* anyway again before the next pread happens. */
+   *needs_clflush = !cpu_cache_is_coherent(obj-base.dev,
+   obj-cache_level);
+   ret = i915_gem_object_wait_rendering(obj, true);
+   if (ret)
+   return ret;
+   }
+
+   ret = i915_gem_object_get_pages(obj);
+   if (ret)
+   return ret;
+
+   i915_gem_object_pin_pages(obj);
+
+   return ret;
+}
+
 /* Per-page copy function for the shmem pread fastpath.
  * Flushes invalid cachelines before reading the target if
  * needs_clflush is set. */
@@ -423,23 +459,10 @@ i915_gem_shmem_pread(struct drm_device *dev,
 
obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj);
 
-   if (!(obj-base.read_domains  I915_GEM_DOMAIN_CPU)) {
-   /* If we're not in the cpu read domain, set ourself into the gtt
-* read domain and manually flush cachelines (if required). This
-* optimizes for the case when the gpu will dirty the data
-* anyway again before the next pread happens. */
-   needs_clflush = !cpu_cache_is_coherent(dev, obj-cache_level);
-   ret = i915_gem_object_wait_rendering(obj, true);
-   if (ret)
-   return ret;
-   }
-
-   ret = i915_gem_object_get_pages(obj);
+   ret = i915_gem_obj_prepare_shmem_read(obj, needs_clflush);
if (ret)
return ret;
 
-   i915_gem_object_pin_pages(obj);
-
offset = args-offset;
 
for_each_sg_page(obj-pages-sgl, sg_iter, obj-pages-nents,
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 08/13] drm/i915: Enable register whitelist checks

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

MI_STORE_REGISTER_MEM, MI_LOAD_REGISTER_MEM, and MI_LOAD_REGISTER_IMM
commands allow userspace access to registers. Only certain registers
should be allowed for such access, so enable checking for those commands.
Each ring gets its own register whitelist.

MI_LOAD_REGISTER_REG on HSW also allows register access but is currently
unused by userspace components. Leave it rejected.

PIPE_CONTROL and MEDIA_VFE_STATE allow register access based on certain
bits being set. Reject those as well.

v2: trailing commas, rebased

OTC-Tracker: AXIA-4631
Change-Id: Ie614a2f0eb2e5917de809e5a17957175d24cc44f
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 21 ++---
 drivers/gpu/drm/i915/i915_reg.h|  3 +++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 353e5cf..4f14a24 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -122,9 +122,12 @@ static const struct drm_i915_cmd_descriptor common_cmds[] 
= {
CMD(  MI_SUSPEND_FLUSH, SMI,F,  1,  S  ),
CMD(  MI_SEMAPHORE_MBOX,SMI,   !F,  0xFF,   R  ),
CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   R  ),
-   CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   R  ),
-   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   R  ),
-   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   W,
+ .reg = { .offset = 1, .mask = 0x007C }   ),
+   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   W,
+ .reg = { .offset = 1, .mask = 0x007C }   ),
+   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   W,
+ .reg = { .offset = 1, .mask = 0x007C }   ),
CMD(  MI_BATCH_BUFFER_START,SMI,   !F,  0xFF,   S  ),
 };
 
@@ -141,9 +144,21 @@ static const struct drm_i915_cmd_descriptor render_cmds[] 
= {
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
CMD(  GFX_OP_3DSTATE_VF_STATISTICS, S3D,F,  1,  S  ),
CMD(  PIPELINE_SELECT,  S3D,F,  1,  S  ),
+   CMD(  MEDIA_VFE_STATE,  S3D,   !F,  0x, B,
+ .bits = {{
+   .offset = 2,
+   .mask = MEDIA_VFE_STATE_MMIO_ACCESS_MASK,
+   .expected = 0,
+ }},  ),
CMD(  GPGPU_OBJECT, S3D,   !F,  0xFF,   S  ),
CMD(  GPGPU_WALKER, S3D,   !F,  0xFF,   S  ),
CMD(  GFX_OP_3DSTATE_SO_DECL_LIST,  S3D,   !F,  0x1FF,  S  ),
+   CMD(  GFX_OP_PIPE_CONTROL(5),   S3D,   !F,  0xFF,   B,
+ .bits = {{
+   .offset = 1,
+   .mask = PIPE_CONTROL_MMIO_WRITE,
+   .expected = 0,
+ }},  ),
 };
 
 static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 87523df..11cca96 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -330,6 +330,7 @@
 #define   DISPLAY_PLANE_B   (120)
 #define GFX_OP_PIPE_CONTROL(len)   ((0x329)|(0x327)|(0x224)|(len-2))
 #define   PIPE_CONTROL_GLOBAL_GTT_IVB  (124) /* gen7+ */
+#define   PIPE_CONTROL_MMIO_WRITE  (123)
 #define   PIPE_CONTROL_CS_STALL(120)
 #define   PIPE_CONTROL_TLB_INVALIDATE  (118)
 #define   PIPE_CONTROL_QW_WRITE(114)
@@ -370,6 +371,8 @@
 
 #define PIPELINE_SELECT
((0x329)|(0x127)|(0x124)|(0x416))
 #define GFX_OP_3DSTATE_VF_STATISTICS   
((0x329)|(0x127)|(0x024)|(0xB16))
+#define MEDIA_VFE_STATE
((0x329)|(0x227)|(0x024)|(0x016))
+#define  MEDIA_VFE_STATE_MMIO_ACCESS_MASK (0x18)
 #define GPGPU_OBJECT   
((0x329)|(0x227)|(0x124)|(0x416))
 #define GPGPU_WALKER   
((0x329)|(0x227)|(0x124)|(0x516))
 #define GFX_OP_3DSTATE_DX9_CONSTANTF_VS \
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 11/13] drm/i915: Reject commands that would store to global HWS page

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

PIPE_CONTROL and MI_FLUSH_DW have bits that would write to the
hardware status page. The driver stores request tracking info
there, so don't let userspace overwrite it.

v2: trailing comma fix, rebased

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 24 +++-
 drivers/gpu/drm/i915/i915_reg.h|  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 1528549..f9aa01a 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -193,7 +193,8 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = 
{
  },
  {
.offset = 1,
-   .mask = PIPE_CONTROL_GLOBAL_GTT_IVB,
+   .mask = (PIPE_CONTROL_GLOBAL_GTT_IVB |
+PIPE_CONTROL_STORE_DATA_INDEX),
.expected = 0,
.condition_offset = 1,
.condition_mask = PIPE_CONTROL_POST_SYNC_OP_MASK,
@@ -242,6 +243,13 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = 
{
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
+ },
+ {
+   .offset = 0,
+   .mask = MI_FLUSH_DW_STORE_INDEX,
+   .expected = 0,
+   .condition_offset = 0,
+   .condition_mask = MI_FLUSH_DW_OP_MASK,
  }},  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   B,
  .bits = {{
@@ -278,6 +286,13 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
+ },
+ {
+   .offset = 0,
+   .mask = MI_FLUSH_DW_STORE_INDEX,
+   .expected = 0,
+   .condition_offset = 0,
+   .condition_mask = MI_FLUSH_DW_OP_MASK,
  }},  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   B,
  .bits = {{
@@ -308,6 +323,13 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = {
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
+ },
+ {
+   .offset = 0,
+   .mask = MI_FLUSH_DW_STORE_INDEX,
+   .expected = 0,
+   .condition_offset = 0,
+   .condition_mask = MI_FLUSH_DW_OP_MASK,
  }},  ),
CMD(  COLOR_BLT,S2D,   !F,  0x3F,   S  ),
CMD(  SRC_COPY_BLT, S2D,   !F,  0x3F,   S  ),
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e683b31..46db649 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -335,6 +335,7 @@
 #define GFX_OP_PIPE_CONTROL(len)   ((0x329)|(0x327)|(0x224)|(len-2))
 #define   PIPE_CONTROL_GLOBAL_GTT_IVB  (124) /* gen7+ */
 #define   PIPE_CONTROL_MMIO_WRITE  (123)
+#define   PIPE_CONTROL_STORE_DATA_INDEX(121)
 #define   PIPE_CONTROL_CS_STALL(120)
 #define   PIPE_CONTROL_TLB_INVALIDATE  (118)
 #define   PIPE_CONTROL_QW_WRITE(114)
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 12/13] drm/i915: Add a CMD_PARSER_VERSION getparam

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

So userspace can query the kernel for command parser support.

v2: Add i915_cmd_parser_get_version(), history log, and kerneldoc

OTC-Tracker: AXIA-4631
Change-Id: I58af650db9f6753c2dcac9c54ab432fd31db302f
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 19 +++
 drivers/gpu/drm/i915/i915_dma.c|  3 +++
 drivers/gpu/drm/i915/i915_drv.h|  1 +
 include/uapi/drm/i915_drm.h|  1 +
 4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index f9aa01a..23c8174 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -897,3 +897,22 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,
 
return ret;
 }
+
+/**
+ * i915_cmd_parser_get_version() - get the cmd parser version number
+ *
+ * The cmd parser maintains a simple increasing integer version number suitable
+ * for passing to userspace clients to determine what operations are permitted.
+ *
+ * Return: the current version number of the cmd parser
+ */
+int i915_cmd_parser_get_version(void)
+{
+   /*
+* Command parser version history
+*
+* 1. Initial version. Checks batches and reports violations, but leaves
+*hardware parsing enabled (so does not allow new use cases).
+*/
+   return 1;
+}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7688abc..14875f5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1017,6 +1017,9 @@ static int i915_getparam(struct drm_device *dev, void 
*data,
case I915_PARAM_HAS_EXEC_HANDLE_LUT:
value = 1;
break;
+   case I915_PARAM_CMD_PARSER_VERSION:
+   value = i915_cmd_parser_get_version();
+   break;
default:
DRM_DEBUG(Unknown parameter %d\n, param-param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 27a48d9..6294d61 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2582,6 +2582,7 @@ void i915_get_extra_instdone(struct drm_device *dev, 
uint32_t *instdone);
 const char *i915_cache_level_str(int type);
 
 /* i915_cmd_parser.c */
+int i915_cmd_parser_get_version(void);
 void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring);
 bool i915_needs_cmd_parser(struct intel_ring_buffer *ring);
 int i915_parse_cmds(struct intel_ring_buffer *ring,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 126bfaa..8a3e4ef00 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_EXEC_NO_RELOC25
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 #define I915_PARAM_HAS_WT   27
+#define I915_PARAM_CMD_PARSER_VERSION   28
 
 typedef struct drm_i915_getparam {
int param;
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 04/13] drm/i915: Reject privileged commands

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

The spec defines most of these commands as privileged. A few others,
like the semaphore mbox command and some display commands, are also
reserved for the driver's use. Subsequent patches relax some of
these restrictions.

v2: Rebased

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 54 --
 drivers/gpu/drm/i915/i915_reg.h|  1 +
 2 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 12241e8..90bbb6d 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -116,27 +116,27 @@
 static const struct drm_i915_cmd_descriptor common_cmds[] = {
CMD(  MI_NOOP,  SMI,F,  1,  S  ),
CMD(  MI_USER_INTERRUPT,SMI,F,  1,  S  ),
-   CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  S  ),
+   CMD(  MI_WAIT_FOR_EVENT,SMI,F,  1,  R  ),
CMD(  MI_ARB_CHECK, SMI,F,  1,  S  ),
CMD(  MI_REPORT_HEAD,   SMI,F,  1,  S  ),
CMD(  MI_SUSPEND_FLUSH, SMI,F,  1,  S  ),
-   CMD(  MI_SEMAPHORE_MBOX,SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_SEMAPHORE_MBOX,SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_STORE_DWORD_INDEX, SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_LOAD_REGISTER_IMM(1),  SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_STORE_REGISTER_MEM(1), SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_LOAD_REGISTER_MEM, SMI,   !F,  0xFF,   R  ),
CMD(  MI_BATCH_BUFFER_START,SMI,   !F,  0xFF,   S  ),
 };
 
 static const struct drm_i915_cmd_descriptor render_cmds[] = {
CMD(  MI_FLUSH, SMI,F,  1,  S  ),
-   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  S  ),
+   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  R  ),
CMD(  MI_PREDICATE, SMI,F,  1,  S  ),
CMD(  MI_TOPOLOGY_FILTER,   SMI,F,  1,  S  ),
-   CMD(  MI_DISPLAY_FLIP,  SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_SET_CONTEXT,   SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_DISPLAY_FLIP,  SMI,   !F,  0xFF,   R  ),
+   CMD(  MI_SET_CONTEXT,   SMI,   !F,  0xFF,   R  ),
CMD(  MI_URB_CLEAR, SMI,   !F,  0xFF,   S  ),
-   CMD(  MI_UPDATE_GTT,SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_UPDATE_GTT,SMI,   !F,  0xFF,   R  ),
CMD(  MI_CLFLUSH,   SMI,   !F,  0x3FF,  S  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
CMD(  GFX_OP_3DSTATE_VF_STATISTICS, S3D,F,  1,  S  ),
@@ -151,7 +151,9 @@ static const struct drm_i915_cmd_descriptor 
hsw_render_cmds[] = {
CMD(  MI_RS_CONTROL,SMI,F,  1,  S  ),
CMD(  MI_URB_ATOMIC_ALLOC,  SMI,F,  1,  S  ),
CMD(  MI_RS_CONTEXT,SMI,F,  1,  S  ),
-   CMD(  MI_LOAD_REGISTER_REG, SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_LOAD_SCAN_LINES_INCL,  SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_LOAD_SCAN_LINES_EXCL,  SMI,   !F,  0x3F,   R  ),
+   CMD(  MI_LOAD_REGISTER_REG, SMI,   !F,  0xFF,   R  ),
CMD(  MI_RS_STORE_DATA_IMM, SMI,   !F,  0xFF,   S  ),
CMD(  MI_LOAD_URB_MEM,  SMI,   !F,  0xFF,   S  ),
CMD(  MI_STORE_URB_MEM, SMI,   !F,  0xFF,   S  ),
@@ -166,8 +168,9 @@ static const struct drm_i915_cmd_descriptor 
hsw_render_cmds[] = {
 };
 
 static const struct drm_i915_cmd_descriptor video_cmds[] = {
-   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  S  ),
+   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  R  ),
CMD(  MI_STORE_DWORD_IMM,   SMI,   !F,  0xFF,   S  ),
+   CMD(  MI_UPDATE_GTT,SMI,   !F,  0x3F,   R  ),
CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   S  ),
/*
 * MFX_WAIT doesn't fit the way we handle length for most commands.
@@ -178,18 +181,25 @@ static const struct drm_i915_cmd_descriptor video_cmds[] 
= {
 };
 
 static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
-   CMD(  MI_ARB_ON_OFF,SMI,F,  1,  S  ),
+ 

[Intel-gfx] [PATCH 5/6] tests/gem_exec_parse: Test for batches w/o MI_BATCH_BUFFER_END

2014-02-18 Thread bradley . d . volkin
From: Brad Volkin bradley.d.vol...@intel.com

Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
 tests/gem_exec_parse.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 9e90408..004c3bf 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -257,6 +257,15 @@ igt_main
  -EINVAL));
}
 
+   igt_subtest(batch-without-end) {
+   uint32_t noop[1024] = { 0 };
+   igt_assert(
+  exec_batch(fd, handle,
+ noop, sizeof(noop),
+ I915_EXEC_RENDER,
+ -EINVAL));
+   }
+
igt_fixture {
gem_close(fd, handle);
 
-- 
1.8.3.2

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


[Intel-gfx] [PATCH] drm/i915: Accurately track when we mark the hardware as idle/busy

2014-02-18 Thread Chris Wilson
We currently call intel_mark_idle() too often, as we do so as a
side-effect of processing the request queue. However, we the calls to
intel_mark_idle() are expected to be paired with a call to
intel_mark_busy() (or else we try to idle the hardware by accessing
registers that are already disabled). Make the idle/busy tracking
explicit to prevent the multiple calls.

Reported-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h  |8 
 drivers/gpu/drm/i915/i915_gem.c  |4 +---
 drivers/gpu/drm/i915/intel_display.c |   11 +++
 drivers/gpu/drm/i915/intel_drv.h |2 +-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 00222cc..8441c8a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,6 +1134,14 @@ struct i915_gem_mm {
 */
bool interruptible;
 
+   /**
+* Is the GPU currently considered idle, or busy executing userspace
+* requests?  Whilst idle, we attempt to power down the hardware and
+* display clocks. In order to reduce the effect on performance, there
+* is a slight delay before we do so.
+*/
+   bool busy;
+
/** Bit 6 swizzling required for X tiling */
uint32_t bit_6_swizzle_x;
/** Bit 6 swizzling required for Y tiling */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9a40ef5..4525dd7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2315,7 +2315,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
i915_gem_context_reference(request-ctx);
 
request-emitted_jiffies = jiffies;
-   was_empty = list_empty(ring-request_list);
list_add_tail(request-list, ring-request_list);
request-file_priv = NULL;
 
@@ -2336,12 +2335,11 @@ int __i915_add_request(struct intel_ring_buffer *ring,
if (!dev_priv-ums.mm_suspended) {
i915_queue_hangcheck(ring-dev);
 
-   if (was_empty) {
+   if (intel_mark_busy(dev_priv-dev)) {
cancel_delayed_work_sync(dev_priv-mm.idle_work);
queue_delayed_work(dev_priv-wq,
   dev_priv-mm.retire_work,
   round_jiffies_up_relative(HZ));
-   intel_mark_busy(dev_priv-dev);
}
}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e127b23..bfd6396 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8220,8 +8220,14 @@ void intel_mark_busy(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
+   if (dev_priv-mm.busy)
+   return false;
+
hsw_package_c8_gpu_busy(dev_priv);
i915_update_gfx_val(dev_priv);
+   dev_priv-mm.busy = true;
+
+   return true;
 }
 
 void intel_mark_idle(struct drm_device *dev)
@@ -8229,6 +8235,11 @@ void intel_mark_idle(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
 
+   if (!dev_priv-mm.busy)
+   return;
+
+   dev_priv-mm.busy = false;
+
hsw_package_c8_gpu_idle(dev_priv);
 
if (!i915.powersave)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e5e1a5c..4c329e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -656,7 +656,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 const char *intel_output_name(int output);
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
 int intel_pch_rawclk(struct drm_device *dev);
-void intel_mark_busy(struct drm_device *dev);
+bool intel_mark_busy(struct drm_device *dev);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring);
 void intel_mark_idle(struct drm_device *dev);
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH] xf86-video-intel: Unbreak build on OpenBSD

2014-02-18 Thread Mark Kettenis
 Date: Tue, 18 Feb 2014 08:06:36 +
 From: Chris Wilson ch...@chris-wilson.co.uk
 
 On Sat, Feb 15, 2014 at 09:29:42PM +0100, Mark Kettenis wrote:
  Fallout from the backlight helper changes.
 
 Apologies for missing this earlier. I have rearranged the code once
 again to try to push the OpenBSD specifics down into src/backlight.c,
 can you please check what needs to be fixed up now?

No worries.

Here's a new diff against master.

diff --git a/src/backlight.c b/src/backlight.c
index 688819d..3c3f152 100644
--- a/src/backlight.c
+++ b/src/backlight.c
@@ -32,6 +32,7 @@
 #include sys/types.h
 #include sys/wait.h
 #include sys/stat.h
+#include sys/ioctl.h
 
 #include stdio.h
 #include stdlib.h
@@ -71,13 +72,14 @@
 #ifdef __OpenBSD__
 
 #include dev/wscons/wsconsio.h
+#include xf86Priv.h
 
 int backlight_set(struct backlight *b, int level)
 {
struct wsdisplay_param param;
 
if (b-iface == NULL)
-   return;
+   return -1;
 
if ((unsigned)level  b-max)
level = b-max;
@@ -129,6 +131,14 @@ int backlight_open(struct backlight *b, char *iface)
return param.curval;
 }
 
+enum backlight_type backlight_exists(const char *iface)
+{
+   if (iface != NULL)
+   return BL_NONE;
+
+   return BL_PLATFORM;
+}
+
 #else
 
 static int
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 03/11] [v2] drm/i915: Rename and comment all the RPS *stuff*

2014-02-18 Thread Ben Widawsky
The names of the struct members for RPS are stupid. Every time I need to
do anything in this code I have to spend a significant amount of time to
remember what it all means. By renaming the variables (and adding the
comments) I hope to clear up the situation. Indeed doing this make some
upcoming patches more readable.

I've avoided ILK because it's possible that the naming used for Ironlake
matches what is in the docs. I believe the ILK power docs were never
published, and I am too lazy to dig them up.

While there may be mistakes, this patch was mostly done via sed. The
renaming of hw_max required a bit of interactivity.

v2: Updated code comments to be less repetitive and more informative
(Ben)

Cc: Jeff McGee jeff.mc...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_debugfs.c |  26 
 drivers/gpu/drm/i915/i915_drv.h |  31 +++---
 drivers/gpu/drm/i915/i915_irq.c |  24 
 drivers/gpu/drm/i915/i915_sysfs.c   |  44 +++---
 drivers/gpu/drm/i915/intel_pm.c | 116 ++--
 5 files changed, 129 insertions(+), 112 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index d90a707..80087d1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1027,7 +1027,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void 
*unused)
   max_freq * GT_FREQUENCY_MULTIPLIER);
 
seq_printf(m, Max overclocked frequency: %dMHz\n,
-  dev_priv-rps.hw_max * GT_FREQUENCY_MULTIPLIER);
+  dev_priv-rps.max_freq_overclock * 
GT_FREQUENCY_MULTIPLIER);
} else if (IS_VALLEYVIEW(dev)) {
u32 freq_sts, val;
 
@@ -1485,8 +1485,8 @@ static int i915_ring_freq_table(struct seq_file *m, void 
*unused)
 
seq_puts(m, GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring 
freq (MHz)\n);
 
-   for (gpu_freq = dev_priv-rps.min_delay;
-gpu_freq = dev_priv-rps.max_delay;
+   for (gpu_freq = dev_priv-rps.min_freq_softlimit;
+gpu_freq = dev_priv-rps.max_freq_softlimit;
 gpu_freq++) {
ia_freq = gpu_freq;
sandybridge_pcode_read(dev_priv,
@@ -3371,9 +3371,9 @@ i915_max_freq_get(void *data, u64 *val)
return ret;
 
if (IS_VALLEYVIEW(dev))
-   *val = vlv_gpu_freq(dev_priv, dev_priv-rps.max_delay);
+   *val = vlv_gpu_freq(dev_priv, dev_priv-rps.max_freq_softlimit);
else
-   *val = dev_priv-rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+   *val = dev_priv-rps.max_freq_softlimit * 
GT_FREQUENCY_MULTIPLIER;
mutex_unlock(dev_priv-rps.hw_lock);
 
return 0;
@@ -3410,16 +3410,16 @@ i915_max_freq_set(void *data, u64 val)
do_div(val, GT_FREQUENCY_MULTIPLIER);
 
rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-   hw_max = dev_priv-rps.hw_max;
+   hw_max = dev_priv-rps.max_freq_overclock;
hw_min = (rp_state_cap  16)  0xff;
}
 
-   if (val  hw_min || val  hw_max || val  dev_priv-rps.min_delay) {
+   if (val  hw_min || val  hw_max || val  
dev_priv-rps.min_freq_softlimit) {
mutex_unlock(dev_priv-rps.hw_lock);
return -EINVAL;
}
 
-   dev_priv-rps.max_delay = val;
+   dev_priv-rps.max_freq_softlimit = val;
 
if (IS_VALLEYVIEW(dev))
valleyview_set_rps(dev, val);
@@ -3452,9 +3452,9 @@ i915_min_freq_get(void *data, u64 *val)
return ret;
 
if (IS_VALLEYVIEW(dev))
-   *val = vlv_gpu_freq(dev_priv, dev_priv-rps.min_delay);
+   *val = vlv_gpu_freq(dev_priv, dev_priv-rps.min_freq_softlimit);
else
-   *val = dev_priv-rps.min_delay * GT_FREQUENCY_MULTIPLIER;
+   *val = dev_priv-rps.min_freq_softlimit * 
GT_FREQUENCY_MULTIPLIER;
mutex_unlock(dev_priv-rps.hw_lock);
 
return 0;
@@ -3491,16 +3491,16 @@ i915_min_freq_set(void *data, u64 val)
do_div(val, GT_FREQUENCY_MULTIPLIER);
 
rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-   hw_max = dev_priv-rps.hw_max;
+   hw_max = dev_priv-rps.max_freq_overclock;
hw_min = (rp_state_cap  16)  0xff;
}
 
-   if (val  hw_min || val  hw_max || val  dev_priv-rps.max_delay) {
+   if (val  hw_min || val  hw_max || val  
dev_priv-rps.max_freq_softlimit) {
mutex_unlock(dev_priv-rps.hw_lock);
return -EINVAL;
}
 
-   dev_priv-rps.min_delay = val;
+   dev_priv-rps.min_freq_softlimit = val;
 
if (IS_VALLEYVIEW(dev))
valleyview_set_rps(dev, val);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c64831..90aa1c5 100644
--- 

Re: [Intel-gfx] [PATCH] xf86-video-intel: Unbreak build on OpenBSD

2014-02-18 Thread Chris Wilson
On Tue, Feb 18, 2014 at 07:55:46PM +0100, Mark Kettenis wrote:
  Date: Tue, 18 Feb 2014 08:06:36 +
  From: Chris Wilson ch...@chris-wilson.co.uk
  
  On Sat, Feb 15, 2014 at 09:29:42PM +0100, Mark Kettenis wrote:
   Fallout from the backlight helper changes.
  
  Apologies for missing this earlier. I have rearranged the code once
  again to try to push the OpenBSD specifics down into src/backlight.c,
  can you please check what needs to be fixed up now?
 
 No worries.
 
 Here's a new diff against master.

Thank you, pushed.

commit c91af569ee90a832899c9038badd84921e9a87fc
Author: Mark Kettenis mark.kette...@xs4all.nl
Date:   Tue Feb 18 19:02:14 2014 +

backlight: Build fixes for OpenBSD
-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: Fix forcewake counts for gen8

2014-02-18 Thread Mika Kuoppala
Ben Widawsky benjamin.widaw...@intel.com writes:

 On Tue, Feb 18, 2014 at 07:10:24PM +0200, Mika Kuoppala wrote:
 Sometimes generic driver code gets forcewake explicitly by
 gen6_gt_force_wake_get(), which check forcewake_count before accessing
 hardware. However the register access with gen8_write function access
 low level hw accessors directly, ignoring the forcewake_count. This
 leads to nested forcewake get from hardware, in ring init and possibly
 elsewhere, causing forcewake ack clear errors and/or hangs.
 
 Fix this by checking the forcewake count also in gen8_write
 
 v2: Read side doesn't care about shadowed registers,
 Remove __needs_put funkiness from gen8_write. (Ville)
 Improved commit message.
 
 References: https://bugs.freedesktop.org/show_bug.cgi?id=74007
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
  
Double signed-off slipped in it seems.

 Thanks for finding this. I'd been meaning to track down the extra Timed
 out messages. I do wonder how this actually fixes a hang. Do you have
 any idea?

No clear idea. I just suspect that the two writes into FORCEWAKE_MT
without proper put/ack dance in between upsets the gpu.

Interestingly with init it hangs like 1 out of 2 times,
but after a gpu reset, it hangs always. The spot is always
the same: I915_WRITE(mmio, (u32)ring-status_page.gfx_addr);

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


Re: [Intel-gfx] [PATCH v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Igor Gnatenko
On Tue, 2014-02-18 at 16:22 +0100, Rafael J. Wysocki wrote: 
 On Tuesday, February 18, 2014 02:31:46 PM Takashi Iwai wrote:
  At Tue, 18 Feb 2014 12:34:42 +0200,
  Mika Westerberg wrote:
   
   On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
+   {
+   .callback = video_set_use_native_backlight,
+   .ident = HP EliteBook 2013 models,
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
+   DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
+   DMI_MATCH(DMI_PRODUCT_NAME,  G1),
+   },
+   },
   
   I see my device is listed here but the above doesn't really use native
   backlight because it is still in acpi_osi blacklist. Tried this and I can
   see both acpi_video0 and intel_backlight listed under 
   /sys/class/backlight.
   Was this the intention?
  
  The acpi_osi blacklist commit (2d4054d84224) has to be reverted, as I
  mentioned earlier.  But the revert can be done individually after
  merging this patch.  Rafael, could you care?
 
 Done.  Please check the result in linux-pm.git/linux-next.
Is it will merge to 3.14 ?

P.S. why you don't using rafael@ mail ? *joke* 
 
 Thanks,
 Rafael
 


-- 
-Igor Gnatenko

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


[Intel-gfx] [PATCH] drm/i915: honor forced connector modes

2014-02-18 Thread Jesse Barnes
In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off.  Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config.

Reported-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_fbdev.c |   15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
b/drivers/gpu/drm/i915/intel_fbdev.c
index 7693728..7ffab6d 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -293,6 +293,21 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
bool *save_enabled;
bool any_enabled = false;
 
+   /*
+* If the user specified any force options, just bail here
+* and use that config.
+*/
+   for (i = 0; i  fb_helper-connector_count; i++) {
+   struct drm_fb_helper_connector *fb_conn;
+   struct drm_connector *connector;
+
+   fb_conn = fb_helper-connector_info[i];
+   connector = fb_conn-connector;
+
+   if (connector-force != DRM_FORCE_UNSPECIFIED)
+   return false;
+   }
+
save_enabled = kcalloc(dev-mode_config.num_connector, sizeof(bool),
   GFP_KERNEL);
if (!save_enabled)
-- 
1.7.9.5

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


Re: [Intel-gfx] [PATCH] drm/i915: Accurately track when we mark the hardware as idle/busy

2014-02-18 Thread Paulo Zanoni
2014-02-18 16:25 GMT-03:00 Mika Kuoppala mika.kuopp...@linux.intel.com:
 Chris Wilson ch...@chris-wilson.co.uk writes:

 We currently call intel_mark_idle() too often, as we do so as a
 side-effect of processing the request queue. However, we the calls to
 intel_mark_idle() are expected to be paired with a call to
 intel_mark_busy() (or else we try to idle the hardware by accessing
 registers that are already disabled). Make the idle/busy tracking
 explicit to prevent the multiple calls.

 Reported-by: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Paulo Zanoni paulo.r.zan...@intel.com

Thanks!

I tested it and this patch + another local patch I have fix the
problem that can be reproduced by the gem-idle subtest of pm_pc8.c
(I still did not commit the subtest, but will do it soon).

Also, I guess this patch deprecates dev_priv-pc8.gpu_idle. I had
plans to move it to dev_priv-pm.gpu_idle, but now I'll try to kill
it.

 ---
  drivers/gpu/drm/i915/i915_drv.h  |8 
  drivers/gpu/drm/i915/i915_gem.c  |4 +---
  drivers/gpu/drm/i915/intel_display.c |   11 +++
  drivers/gpu/drm/i915/intel_drv.h |2 +-
  4 files changed, 21 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h 
 b/drivers/gpu/drm/i915/i915_drv.h
 index 00222cc..8441c8a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1134,6 +1134,14 @@ struct i915_gem_mm {
*/
   bool interruptible;

 + /**
 +  * Is the GPU currently considered idle, or busy executing userspace
 +  * requests?  Whilst idle, we attempt to power down the hardware and
 +  * display clocks. In order to reduce the effect on performance, there
 +  * is a slight delay before we do so.
 +  */
 + bool busy;
 +
   /** Bit 6 swizzling required for X tiling */
   uint32_t bit_6_swizzle_x;
   /** Bit 6 swizzling required for Y tiling */
 diff --git a/drivers/gpu/drm/i915/i915_gem.c 
 b/drivers/gpu/drm/i915/i915_gem.c
 index 9a40ef5..4525dd7 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2315,7 +2315,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
   i915_gem_context_reference(request-ctx);

   request-emitted_jiffies = jiffies;
 - was_empty = list_empty(ring-request_list);
   list_add_tail(request-list, ring-request_list);
   request-file_priv = NULL;

 @@ -2336,12 +2335,11 @@ int __i915_add_request(struct intel_ring_buffer 
 *ring,
   if (!dev_priv-ums.mm_suspended) {
   i915_queue_hangcheck(ring-dev);

 - if (was_empty) {
 + if (intel_mark_busy(dev_priv-dev)) {

I'm new to this code, so forgive me if I'm way off. Now that we
changed the relative order, isn't it possible that we run the code
line above, marking the device as busy, and then just before the next
line runs, the still-not-canceled idle_work function runs and marks
the device as idle? That could be bad, right?

Also, why do we need the change on this function?


   cancel_delayed_work_sync(dev_priv-mm.idle_work);
   queue_delayed_work(dev_priv-wq,
  dev_priv-mm.retire_work,
  round_jiffies_up_relative(HZ));
 - intel_mark_busy(dev_priv-dev);
   }
   }

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index e127b23..bfd6396 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8220,8 +8220,14 @@ void intel_mark_busy(

 bool intel_mark_busy(struct drm_device *dev)

 -Mika

Exactly.

Thanks,
Paulo


  {
   struct drm_i915_private *dev_priv = dev-dev_private;

 + if (dev_priv-mm.busy)
 + return false;
 +
   hsw_package_c8_gpu_busy(dev_priv);
   i915_update_gfx_val(dev_priv);
 + dev_priv-mm.busy = true;
 +
 + return true;
  }

  void intel_mark_idle(struct drm_device *dev)
 @@ -8229,6 +8235,11 @@ void intel_mark_idle(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;

 + if (!dev_priv-mm.busy)
 + return;
 +
 + dev_priv-mm.busy = false;
 +
   hsw_package_c8_gpu_idle(dev_priv);

   if (!i915.powersave)
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index e5e1a5c..4c329e0 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -656,7 +656,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
  const char *intel_output_name(int output);
  bool intel_has_pending_fb_unpin(struct drm_device *dev);
  int intel_pch_rawclk(struct drm_device *dev);
 -void intel_mark_busy(struct drm_device *dev);
 +bool intel_mark_busy(struct drm_device *dev);
  void intel_mark_fb_busy(struct 

Re: [Intel-gfx] [PATCH v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Rafael J. Wysocki
On Tuesday, February 18, 2014 11:28:29 PM Igor Gnatenko wrote:
 On Tue, 2014-02-18 at 16:22 +0100, Rafael J. Wysocki wrote: 
  On Tuesday, February 18, 2014 02:31:46 PM Takashi Iwai wrote:
   At Tue, 18 Feb 2014 12:34:42 +0200,
   Mika Westerberg wrote:

On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
 + {
 + .callback = video_set_use_native_backlight,
 + .ident = HP EliteBook 2013 models,
 + .matches = {
 + DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
 + DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
 + DMI_MATCH(DMI_PRODUCT_NAME,  G1),
 + },
 + },

I see my device is listed here but the above doesn't really use native
backlight because it is still in acpi_osi blacklist. Tried this and I 
can
see both acpi_video0 and intel_backlight listed under 
/sys/class/backlight.
Was this the intention?
   
   The acpi_osi blacklist commit (2d4054d84224) has to be reverted, as I
   mentioned earlier.  But the revert can be done individually after
   merging this patch.  Rafael, could you care?
  
  Done.  Please check the result in linux-pm.git/linux-next.
 Is it will merge to 3.14 ?

That's the plan.

 P.S. why you don't using rafael@ mail ? *joke* 

Because it is hooked up to gmail.  And I actually do use it sometimes.

Thanks,
Rafael

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


Re: [Intel-gfx] [PATCH] drm/i915: Accurately track when we mark the hardware as idle/busy

2014-02-18 Thread Paulo Zanoni
2014-02-18 18:34 GMT-03:00 Paulo Zanoni przan...@gmail.com:
 2014-02-18 16:25 GMT-03:00 Mika Kuoppala mika.kuopp...@linux.intel.com:
 Chris Wilson ch...@chris-wilson.co.uk writes:

 We currently call intel_mark_idle() too often, as we do so as a
 side-effect of processing the request queue. However, we the calls to
 intel_mark_idle() are expected to be paired with a call to
 intel_mark_busy() (or else we try to idle the hardware by accessing
 registers that are already disabled). Make the idle/busy tracking
 explicit to prevent the multiple calls.

 Reported-by: Paulo Zanoni paulo.r.zan...@intel.com
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Paulo Zanoni paulo.r.zan...@intel.com

 Thanks!

 I tested it and this patch + another local patch I have fix the
 problem that can be reproduced by the gem-idle subtest of pm_pc8.c
 (I still did not commit the subtest, but will do it soon).

 Also, I guess this patch deprecates dev_priv-pc8.gpu_idle. I had
 plans to move it to dev_priv-pm.gpu_idle, but now I'll try to kill
 it.

 ---
  drivers/gpu/drm/i915/i915_drv.h  |8 
  drivers/gpu/drm/i915/i915_gem.c  |4 +---
  drivers/gpu/drm/i915/intel_display.c |   11 +++
  drivers/gpu/drm/i915/intel_drv.h |2 +-
  4 files changed, 21 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h 
 b/drivers/gpu/drm/i915/i915_drv.h
 index 00222cc..8441c8a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1134,6 +1134,14 @@ struct i915_gem_mm {
*/
   bool interruptible;

 + /**
 +  * Is the GPU currently considered idle, or busy executing userspace
 +  * requests?  Whilst idle, we attempt to power down the hardware and
 +  * display clocks. In order to reduce the effect on performance, there
 +  * is a slight delay before we do so.
 +  */
 + bool busy;

Hi

Also, don't we want to init this to true, since the first thing called
seems to be intel_mark_idle?

I get intel_mark_idle called at 6 seconds after booting, then
intel_mark_busy is called only 19 seconds after booting.

I found this while writing the patch to deprecate dev_priv-pc8.gpu_idle :)

Thanks,
Paulo

 +
   /** Bit 6 swizzling required for X tiling */
   uint32_t bit_6_swizzle_x;
   /** Bit 6 swizzling required for Y tiling */
 diff --git a/drivers/gpu/drm/i915/i915_gem.c 
 b/drivers/gpu/drm/i915/i915_gem.c
 index 9a40ef5..4525dd7 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2315,7 +2315,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
   i915_gem_context_reference(request-ctx);

   request-emitted_jiffies = jiffies;
 - was_empty = list_empty(ring-request_list);
   list_add_tail(request-list, ring-request_list);
   request-file_priv = NULL;

 @@ -2336,12 +2335,11 @@ int __i915_add_request(struct intel_ring_buffer 
 *ring,
   if (!dev_priv-ums.mm_suspended) {
   i915_queue_hangcheck(ring-dev);

 - if (was_empty) {
 + if (intel_mark_busy(dev_priv-dev)) {

 I'm new to this code, so forgive me if I'm way off. Now that we
 changed the relative order, isn't it possible that we run the code
 line above, marking the device as busy, and then just before the next
 line runs, the still-not-canceled idle_work function runs and marks
 the device as idle? That could be bad, right?

 Also, why do we need the change on this function?


   cancel_delayed_work_sync(dev_priv-mm.idle_work);
   queue_delayed_work(dev_priv-wq,
  dev_priv-mm.retire_work,
  round_jiffies_up_relative(HZ));
 - intel_mark_busy(dev_priv-dev);
   }
   }

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index e127b23..bfd6396 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8220,8 +8220,14 @@ void intel_mark_busy(

 bool intel_mark_busy(struct drm_device *dev)

 -Mika

 Exactly.

 Thanks,
 Paulo


  {
   struct drm_i915_private *dev_priv = dev-dev_private;

 + if (dev_priv-mm.busy)
 + return false;
 +
   hsw_package_c8_gpu_busy(dev_priv);
   i915_update_gfx_val(dev_priv);
 + dev_priv-mm.busy = true;
 +
 + return true;
  }

  void intel_mark_idle(struct drm_device *dev)
 @@ -8229,6 +8235,11 @@ void intel_mark_idle(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;

 + if (!dev_priv-mm.busy)
 + return;
 +
 + dev_priv-mm.busy = false;
 +
   hsw_package_c8_gpu_idle(dev_priv);

   if (!i915.powersave)
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index e5e1a5c..4c329e0 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ 

Re: [Intel-gfx] [PATCH v4] ACPI / video: Add systems that should favour native backlight interface

2014-02-18 Thread Aaron Lu
On 02/18/2014 11:46 PM, Mika Westerberg wrote:
 On Tue, Feb 18, 2014 at 04:22:27PM +0100, Rafael J. Wysocki wrote:
 On Tuesday, February 18, 2014 02:31:46 PM Takashi Iwai wrote:
 At Tue, 18 Feb 2014 12:34:42 +0200,
 Mika Westerberg wrote:

 On Tue, Feb 18, 2014 at 01:54:20PM +0800, Aaron Lu wrote:
 + {
 + .callback = video_set_use_native_backlight,
 + .ident = HP EliteBook 2013 models,
 + .matches = {
 + DMI_MATCH(DMI_SYS_VENDOR, Hewlett-Packard),
 + DMI_MATCH(DMI_PRODUCT_NAME, HP EliteBook ),
 + DMI_MATCH(DMI_PRODUCT_NAME,  G1),
 + },
 + },

 I see my device is listed here but the above doesn't really use native
 backlight because it is still in acpi_osi blacklist. Tried this and I can
 see both acpi_video0 and intel_backlight listed under /sys/class/backlight.
 Was this the intention?

 The acpi_osi blacklist commit (2d4054d84224) has to be reverted, as I
 mentioned earlier.  But the revert can be done individually after
 merging this patch.  Rafael, could you care?

 Done.  Please check the result in linux-pm.git/linux-next.
 
 With your revert and this patch from Aaron, backlight on my HP EliteBook
 Revolve G1 works fine, thanks!
 
 Tested-by: Mika Westerberg mika.westerb...@linux.intel.com

Thanks for the test Mika and sorry for all the confusions caused.

-Aaron

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


[Intel-gfx] Request for feedback : New Panel-fitter property for connectors

2014-02-18 Thread Goel, Akash

Hello, 

We are planning to expose a new panel fitter property for the connectors. 
We have some use cases like a full screen video playback, where we can flip the 
frame buffer of any size irrespective of the pipe timings and enable the panel 
fitter to do scaling instead of using the GPU. 
Other use case is the Clone mode, where we can use the composed output of the 
LFP on the external display, with panel fitter enabled, to avoid GPU 
composition.

Please provide inputs/comments on the same.

Best regards
Akash

-Original Message-
From: Goel, Akash 
Sent: Tuesday, February 18, 2014 4:12 PM
To: intel-gfx@lists.freedesktop.org
Cc: Goel, Akash; G, Pallavi
Subject: [PATCH] drm/i915: Added a new Panel-fitter property for connectors

From: Akash Goel akash.g...@intel.com

Added a Panel-fitter enable/disable property support for the connectors.

NOTE: By default, the value for the property will be disabled.

Signed-off-by: G Pallavi pallav...@intel.com
Signed-off-by: Akash Goel akash.g...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  8 
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 31 +++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h 
index 8c64831..a07f6cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1560,6 +1560,7 @@ typedef struct drm_i915_private {
 
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+   struct drm_property *force_pfit_property;
 
uint32_t hw_context_size;
struct list_head context_list;
@@ -1613,6 +1614,13 @@ enum hdmi_force_audio {
HDMI_AUDIO_ON,  /* force turn on HDMI audio */
 };
 
+enum panel_fitter {
+   PFIT_OFF,
+   AUTOSCALE,
+   PILLARBOX,
+   LETTERBOX,
+};
+
 #define I915_GTT_OFFSET_NONE ((u32)-1)
 
 struct drm_i915_gem_object_ops {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4ffc02..db6ea3e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -819,6 +819,7 @@ int intel_connector_update_modes(struct drm_connector 
*connector,  int intel_ddc_get_modes(struct drm_connector *c, struct 
i2c_adapter *adapter);  void intel_attach_force_audio_property(struct 
drm_connector *connector);  void intel_attach_broadcast_rgb_property(struct 
drm_connector *connector);
+void intel_attach_force_pfit_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c 
b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..967e080 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -28,6 +28,7 @@
 #include linux/fb.h
 #include drm/drm_edid.h
 #include drm/drmP.h
+#include drm/drm_crtc.h
 #include intel_drv.h
 #include i915_drv.h
 
@@ -126,3 +127,33 @@ intel_attach_broadcast_rgb_property(struct drm_connector 
*connector)
 
drm_object_attach_property(connector-base, prop, 0);  }
+
+static const struct drm_prop_enum_list pfit_names[] = {
+   { PFIT_OFF, pfit off },
+   { AUTO_SCALE, Auto scale },
+   { PILLAR_BOX, PillarBox },
+   { LETTER_BOX, LetterBox },
+};
+
+void
+intel_attach_force_pfit_property(struct drm_connector *connector) {
+   struct drm_device *dev = connector-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct drm_property *prop;
+   struct drm_mode_object *obj = connector-base;
+
+   prop = dev_priv-force_pfit_property;
+   if (prop == NULL) {
+   prop = drm_property_create_enum(dev, 0,
+   pfit,
+   pfit_names,
+   ARRAY_SIZE(pfit_names));
+   if (prop == NULL)
+   return;
+
+   dev_priv-force_pfit_property = prop;
+   }
+
+   drm_object_attach_property(obj, prop, 0); }
--
1.8.5.2

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


Re: [Intel-gfx] [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices

2014-02-18 Thread Aaron Lu
On 02/13/2014 08:03 PM, Daniel Vetter wrote:
 On Thu, Feb 13, 2014 at 11:08 AM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
 On Thu, Feb 13, 2014 at 05:10:25PM +0800, Aaron Lu wrote:
 On 02/12/2014 06:31 PM, Chris Wilson wrote:
 On Wed, Feb 12, 2014 at 11:05:40AM +0800, Aaron Lu wrote:
 The ACPI table on ASUS UX302LA has more than 8 output devices under the
 graphics controller device node. The problem is, the real active output
 device, the LCD panel, is listed the last. The result is, the LCD's
 device id doesn't get recorded in the active device list CADL array and
 when the _DCS control method for the LCD device is executed, it returns
 0x1d, meaning it is not active. This affects the hotkey delivery ASL
 code that will not deliver a notification if the output device is not
 active on backlight hotkey press.

 I don't see a clean way to solve this problem since the operation region
 spec doesn't allow more than 8 output devices so we have no way of
 storing all these output devices. The fact that output devices that have
 _BCM control method usually means they have a higher possibility of being
 used than those who don't made me choose a simple way to work around
 the buggy firmware by replacing the last entry in CADL array with the one
 that has _BCM control method. There is no specific reason why the last
 entry is picked instead of others.

 Another possibility is that the connector list is in rough priority
 order so might be useful for sorting the CADL array.

 Since the CADL should only be a list of currently active devices, we
 could just bite the bullet and repopulate it correctly after every
 setcrtc.

 Thanks for the suggestion. As a first step, does the following un-tested
 patch look OK?

 Yes. Maybe worth putting together the similar routines for blind
 setting the didl and the cadl, or at least for computing the value from
 the connector. For instance, the didl logic disagrees with the value of
 index - is that relevant? I have a suspicion that the CADL entry should
 match the DIDL entry for the connector, but that is not actually
 mentioned in the opregion spec afaict.
 
 I think a problem is that often we have more than one output for a
 given type. So we need to somehow match them up to make sure we put
 the right ones intot didl/cadl lists. The issue here is that our
 connectors don't match up perfectly with the acpi output entries
 (since we have separate dp/hdmi outputs). But I think it would be
 worthwhile trying to match them up and store a link from struct
 intel_connector to the right acpi node acpi node.

Is this possible? I don't see a way to match them up...
The _ADR control method uses a field that seems to be assigned by BIOS
like this:

Device (LCDD)
{
  Method (_ADR, 0, Serialized)  // _ADR: Address
{
Return (And (0x, DID2))
}
}
DID2 is in system memory region and has some assigned value like 0x400
when we read it. For this case it is easy since there is only one output
device that is of type LVDS so we can match it to connector of type eDP
or LVDS, suppose there is only one such connector. But for output
devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea
how to match them up to the connectors of that type as we can't be sure
the probe order we have used in i915 driver is the same as BIOS'.

In the mean time, it seems we can just use driver's information to
populate both DIDL and CADL and forget the _ADR of those devices like
the following patch does:

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4ffc021c317..55956a517a77 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -204,6 +204,9 @@ struct intel_connector {
/* since POLL and HPD connectors may use the same HPD line keep the 
native
   state of connector-polled in case hotplug storm detection changes 
it */
u8 polled;
+
+   /* device id with type and index information */
+   u32 disp_id;
 };
 
 typedef struct dpll {
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 68459605bd12..ba08c894ce9a 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -221,11 +221,11 @@ struct opregion_asle {
 #define SWSCI_SBCB_POST_VBE_PM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
 #define SWSCI_SBCB_ENABLE_DISABLE_AUDIOSWSCI_FUNCTION_CODE(SWSCI_SBCB, 
21)
 
-#define ACPI_OTHER_OUTPUT (08)
-#define ACPI_VGA_OUTPUT (18)
-#define ACPI_TV_OUTPUT (28)
-#define ACPI_DIGITAL_OUTPUT (38)
-#define ACPI_LVDS_OUTPUT (48)
+#define ACPI_OTHER_OUTPUT 0
+#define ACPI_VGA_OUTPUT 1
+#define ACPI_TV_OUTPUT 2
+#define ACPI_DIGITAL_OUTPUT 3
+#define ACPI_LVDS_OUTPUT 4
 
 #define MAX_DSLP   1500
 
@@ -600,78 +600,20 @@ static struct notifier_block intel_opregion_notifier = {

Re: [Intel-gfx] [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices

2014-02-18 Thread Matthew Garrett
On Wed, Feb 19, 2014 at 03:31:29PM +0800, Aaron Lu wrote:

 DID2 is in system memory region and has some assigned value like 0x400
 when we read it. For this case it is easy since there is only one output
 device that is of type LVDS so we can match it to connector of type eDP
 or LVDS, suppose there is only one such connector. But for output
 devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea
 how to match them up to the connectors of that type as we can't be sure
 the probe order we have used in i915 driver is the same as BIOS'.

Non-standard _ADR values are assigend by the GPU vendor, so Intel should 
be able to provide you with the correct interpretations.

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