Re: [Intel-gfx] [PATCH] xf86-video-intel: Unbreak build on OpenBSD
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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*
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
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
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
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
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 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
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 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
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
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
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
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