Re: [PATCH 1/2] drm/i915: Don't clobber the addfb2 ioctl params
On Wed, Nov 11, 2015 at 07:11:28PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We try to convert the old way of of specifying fb tiling (obj->tiling)
> into the new fb modifiers. We store the result in the passed in mode_cmd
> structure. But that structure comes directly from the addfb2 ioctl, and
> gets copied back out to userspace, which means we're clobbering the
> modifiers that the user provided (all 0 since the DRM_MODE_FB_MODIFIERS
> flag wasn't even set by the user). Hence if the user reuses the struct
> for another addfb2, the ioctl will be rejected since it's now asking for
> some modifiers w/o the flag set.
>
> Fix the problem by making a copy of the user provided structure. We can
> play any games we want with the copy.
>
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter
> Cc: Tvrtko Ursulin
> Fixes: 2a80eada326f ("drm/i915: Add fb format modifier support")
> Testcase: igt/kms_addfb_basic/clobbered-modifier
> Signed-off-by: Ville Syrjälä
Out of curiosity: Where does this blow up? That should be added to the
commit message (so that people affected can match it up with this fix).
With that:
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_display.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b5f7493..c3aa6f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14578,17 +14578,18 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> static struct drm_framebuffer *
> intel_user_framebuffer_create(struct drm_device *dev,
> struct drm_file *filp,
> - struct drm_mode_fb_cmd2 *mode_cmd)
> + struct drm_mode_fb_cmd2 *user_mode_cmd)
> {
> struct drm_framebuffer *fb;
> struct drm_i915_gem_object *obj;
> + struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
>
> obj = to_intel_bo(drm_gem_object_lookup(dev, filp,
> - mode_cmd->handles[0]));
> + mode_cmd.handles[0]));
> if (&obj->base == NULL)
> return ERR_PTR(-ENOENT);
>
> - fb = intel_framebuffer_create(dev, mode_cmd, obj);
> + fb = intel_framebuffer_create(dev, &mode_cmd, obj);
> if (IS_ERR(fb))
> drm_gem_object_unreference_unlocked(&obj->base);
>
> --
> 2.4.10
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drm/core: Fix old_fb handling in drm_mode_atomic_ioctl.
On Wed, Nov 11, 2015 at 11:29:08AM +0100, Maarten Lankhorst wrote: > From: Maarten Lankhorst > > plane_mask should be cleared inside the retry loop, > because it gets reset on every retry. > > Signed-off-by: Maarten Lankhorst > Cc: stable@vger.kernel.org #v4.3 Nice catch, but a bit a terse commit message. We should add "Without this fix the plane->fb refcounting might get out of sync on retries, resulting in either leaked memory or use-after-free." With that: Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 7bb3845d9974..0ac31b1ecb67 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1446,7 +1446,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > struct drm_plane *plane; > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > - unsigned plane_mask = 0; > + unsigned plane_mask; > int ret = 0; > unsigned int i, j; > > @@ -1486,6 +1486,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); > > retry: > + plane_mask = 0; > copied_objs = 0; > copied_props = 0; > > -- > 2.1.0 > > _______ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] drm: Fix primary plane size for stereo doubled modes for legacy setcrtc
On Mon, Nov 16, 2015 at 05:02:34PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Properly double the hdisplay/vdisplay timings that we use as the primary
> plane size with stereo doubled modes. Otherwise the modeset gets
> rejected on machines where the primary plane must be fullscreen, and on
> the rest only the first eye would get a visible plane.
>
> Cc: Daniel Vetter
> Cc: stable@vger.kernel.org
> Fixes: 042652ed9599 ("drm/atomic-helper: implementatations for legacy
> interfaces")
> Signed-off-by: Ville Syrjälä
Testcase: igt/kms_3d
Reviewed-by: Daniel Vetter
To avoid conflicts with patches 2-4 I applied all of them to drm-misc, but
we need to cherry-pick this one to drm-fixes too.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 9870c70..7857163 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1741,6 +1741,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set
> *set,
> struct drm_crtc_state *crtc_state;
> struct drm_plane_state *primary_state;
> struct drm_crtc *crtc = set->crtc;
> + int hdisplay, vdisplay;
> int ret;
>
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> @@ -1783,19 +1784,21 @@ int __drm_atomic_helper_set_config(struct
> drm_mode_set *set,
> if (ret != 0)
> return ret;
>
> + drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
> +
> drm_atomic_set_fb_for_plane(primary_state, set->fb);
> primary_state->crtc_x = 0;
> primary_state->crtc_y = 0;
> - primary_state->crtc_h = set->mode->vdisplay;
> - primary_state->crtc_w = set->mode->hdisplay;
> + primary_state->crtc_h = vdisplay;
> + primary_state->crtc_w = hdisplay;
> primary_state->src_x = set->x << 16;
> primary_state->src_y = set->y << 16;
> if (primary_state->rotation & (BIT(DRM_ROTATE_90) |
> BIT(DRM_ROTATE_270))) {
> - primary_state->src_h = set->mode->hdisplay << 16;
> - primary_state->src_w = set->mode->vdisplay << 16;
> + primary_state->src_h = hdisplay << 16;
> + primary_state->src_w = vdisplay << 16;
> } else {
> - primary_state->src_h = set->mode->vdisplay << 16;
> - primary_state->src_w = set->mode->hdisplay << 16;
> + primary_state->src_h = vdisplay << 16;
> + primary_state->src_w = hdisplay << 16;
> }
>
> commit:
> --
> 2.4.10
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Unpin framebuffers when crtc is deconfigured.
On Wed, Nov 04, 2015 at 02:43:31PM +0100, Maarten Lankhorst wrote:
> When setcrtc is called and steals the last connector away from a crtc
> it's turned off because it can't stay configured without connectors.
>
> The framebuffer is still preserved however, and that causes troubles
> in the IGT stress test kms_flip.flips-vs-fences which tries to use
> as many pins as possible and hangs on the third crtc because of
> framebuffer pins on the first 2 crtc's.
>
> Cc: stable@vger.kernel.org #v4.3
> Signed-off-by: Maarten Lankhorst
drm_atomic_helper_set_config clears the fb for the primary plane if we
clear the mode. Where is the leak?
Also where exactly does it hang - we should have plenty of fences left for
3x2 pinned framebuffers. If you allow more than 2 pinned fb per plane then
that's a bug somewhere with pageflips getting ahead of the unpin worker.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5e520ae5f42e..d95d8acae51f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13571,15 +13571,17 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
> + struct drm_crtc_state *crtc_state;
> int ret = 0;
>
> if (!obj && !old_obj)
> return 0;
>
> - if (old_obj) {
> - struct drm_crtc_state *crtc_state =
> - drm_atomic_get_existing_crtc_state(new_state->state,
> plane->state->crtc);
> + crtc_state = drm_atomic_get_existing_crtc_state(new_state->state,
> + new_state->crtc ?:
> + plane->state->crtc);
>
> + if (old_obj) {
> /* Big Hammer, we also need to ensure that any pending
>* MI_WAIT_FOR_EVENT inside a user batch buffer on the
>* current scanout is retired before unpinning the old
> @@ -13599,7 +13601,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> return ret;
> }
>
> - if (!obj) {
> + if (!obj || !crtc_state->enable) {
> ret = 0;
> } else if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> INTEL_INFO(dev)->cursor_needs_physical) {
> @@ -13644,15 +13646,22 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> struct intel_plane_state *old_intel_state;
> struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
> struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
> + struct drm_crtc_state *old_crtc_state;
>
> old_intel_state = to_intel_plane_state(old_state);
>
> if (!obj && !old_obj)
> return;
>
> - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
> - !INTEL_INFO(dev)->cursor_needs_physical))
> + old_crtc_state = drm_atomic_get_existing_crtc_state(old_state->state,
> + old_state->crtc ?:
> + plane->state->crtc);
> +
> + if (old_obj && old_crtc_state->enable &&
> + (plane->type != DRM_PLANE_TYPE_CURSOR ||
> + !INTEL_INFO(dev)->cursor_needs_physical)) {
> intel_unpin_fb_obj(old_state->fb, old_state);
> + }
>
> /* prepare_fb aborted? */
> if ((old_obj && (old_obj->frontbuffer_bits &
> intel_plane->frontbuffer_bit)) ||
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Mark uneven memory banks on gen4 desktop as unknown swizzling
On Tue, Nov 17, 2015 at 10:40:51AM +, Chris Wilson wrote:
> We have varied reports of swizzling corruption on gen4 desktop, and
> confirmation that it is triggered by uneven memory banks. The
> implication is that the swizzling various between the paired channels
> and the remainder of memory on the single channel. As the object then
> has unpredictable swizzling (it will vary depending on exact page
> allocation and may even change during the object's lifetime as the pages
> are replaced), we have to report to userspace that the swizzling is
> unknown.
>
> Reported-by: Matti Hämäläinen
> References: https://bugs.freedesktop.org/show_bug.cgi?id=90725
> Signed-off-by: Chris Wilson
> Cc: Matti Hämäläinen
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/i915_gem_fence.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c
> b/drivers/gpu/drm/i915/i915_gem_fence.c
> index 40a10b25956c..6ba68083aa95 100644
> --- a/drivers/gpu/drm/i915/i915_gem_fence.c
> +++ b/drivers/gpu/drm/i915/i915_gem_fence.c
> @@ -677,8 +677,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>* the minimum size of a rank.
>*/
> if (I915_READ16(C0DRB3) != I915_READ16(C1DRB3)) {
> - swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> - swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
> + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
Existing userspace goes boom if we hand it _UNKNOWN as the swizzle type.
We need the same lie as with L-shaped on g4x and only set the quirk, so
that phys_swizzle_mode is _UNKOWN. See
commit 5eb3e5a5e11d14f9deb2a4b83555443b69ab9940
Author: Chris Wilson
Date: Sun Jun 28 09:19:26 2015 +0100
drm/i915: Declare the swizzling unknown for L-shaped configurations
So if you keep the swizzle put add the
dev_priv->quirks |= QUIRK_PIN_SWIZZLED_PAGES;
here instead (and drop patch 2) this looks good and gets my r-b. But
please include a reference to the above commit so we don't forget all
this.
-Daniel
> } else {
> swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> swizzle_y = I915_BIT_6_SWIZZLE_9;
> --
> 2.6.2
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] drm/i915: Mark uneven memory banks on gen4 desktop as unknown swizzling
On Thu, Nov 19, 2015 at 09:58:05AM +, Chris Wilson wrote:
> We have varied reports of swizzling corruption on gen4 desktop, and
> confirmation that one at least is triggered by uneven memory banks
> (L-shaped memory). The implication is that the swizzling varies between
> the paired channels and the remainder of memory on the single channel. As
> the object then has unpredictable swizzling (it will vary depending on
> exact page allocation and may even change during the object's lifetime as
> the pages are replaced), we have to report to userspace that the swizzling
> is unknown.
>
> However, some existing userspace is buggy when it meets an unknown
> swizzling configuration and so we need to tell another white lie and
> mark the swizzling as NONE but report it as UNKNOWN through the extended
> get-tiling-ioctl. See
>
> commit 5eb3e5a5e11d14f9deb2a4b83555443b69ab9940
> Author: Chris Wilson
> Date: Sun Jun 28 09:19:26 2015 +0100
>
> drm/i915: Declare the swizzling unknown for L-shaped configurations
>
> for the previous example where we found that telling the truth to
> userspace just ends up in a world of hurt.
>
> Also since we don't truly know what the swizzling is on the pages, we
> need to keep them pinned to prevent swapping as the reports also
> suggest that some gen4 devices have previously undetected bit17
> swizzling.
>
> v2: Combine unknown + quirk patches to prevent userspace ever seeing
> unknown swizzling through the normal get-tiling-ioctl. Also use the same
> path for the existing uneven bank detection for mobile gen4.
>
> Reported-by: Matti Hämäläinen
> Tested--by: Matti Hämäläinen
> References: https://bugs.freedesktop.org/show_bug.cgi?id=90725
> Signed-off-by: Chris Wilson
> Cc: Matti Hämäläinen
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Cc: stable@vger.kernel.org
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem_fence.c | 36
> ++-
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c
> b/drivers/gpu/drm/i915/i915_gem_fence.c
> index 40a10b25956c..f010391b87f5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_fence.c
> +++ b/drivers/gpu/drm/i915/i915_gem_fence.c
> @@ -642,11 +642,10 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
> }
>
> /* check for L-shaped memory aka modified enhanced addressing */
> - if (IS_GEN4(dev)) {
> - uint32_t ddc2 = I915_READ(DCC2);
> -
> - if (!(ddc2 & DCC2_MODIFIED_ENHANCED_DISABLE))
> - dev_priv->quirks |= QUIRK_PIN_SWIZZLED_PAGES;
> + if (IS_GEN4(dev) &&
> + !(I915_READ(DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) {
> + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
> + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
> }
>
> if (dcc == 0x) {
> @@ -675,16 +674,35 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>* matching, which was the case for the swizzling required in
>* the table above, or from the 1-ch value being less than
>* the minimum size of a rank.
> + *
> + * Reports indicate that the swizzling actually
> + * varies depending upon page placement inside the
> + * channels, i.e. we see swizzled pages where the
> + * banks of memory are paired and unswizzled on the
> + * uneven portion, so leave that as unknown.
>*/
> - if (I915_READ16(C0DRB3) != I915_READ16(C1DRB3)) {
> - swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> - swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> - } else {
> + if (I915_READ16(C0DRB3) == I915_READ16(C1DRB3)) {
> swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> swizzle_y = I915_BIT_6_SWIZZLE_9;
> }
> }
>
> + if (swizzle_x == I915_BIT_6_SWIZZLE_UNKNOWN ||
> + swizzle_y == I915_BIT_6_SWIZZLE_UNKNOWN) {
> + /* Userspace likes to explode if it sees unknown swizzling,
> + * so lie. We will finish the lie when reporting through
> + * the get-tiling-ioctl by reporting the physical swizzle
> + * mode as unknown instead.
> + *
> + * As we don't strictly know what the swizzling is, it may be
> + * bit17 dependent, and so we need to also prevent the pages
> + * from being moved.
> + */
> +
Re: [Intel-gfx] [PATCH v4] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping
On Sat, Nov 21, 2015 at 02:01:55AM +0800, kbuild test robot wrote:
> Hi Chris,
>
> [auto build test WARNING on drm-intel/for-linux-next]
> [cannot apply to v4.4-rc1 next-20151120]
>
> url:
> https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-Pin-the-ifbdev-for-the-info-system_base-GGTT-mmapping/20151121-003300
> base: git://anongit.freedesktop.org/drm-intel for-linux-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> drivers/gpu/drm/i915/intel_fbdev.c:225:58: sparse: incorrect type in
> >> argument 2 (different base types)
>drivers/gpu/drm/i915/intel_fbdev.c:225:58:expected struct
> drm_framebuffer *fb
>drivers/gpu/drm/i915/intel_fbdev.c:225:58:got struct drm_framebuffer
> base
>drivers/gpu/drm/i915/intel_fbdev.c: In function 'intelfb_create':
>drivers/gpu/drm/i915/intel_fbdev.c:225:41: error: incompatible type for
> argument 2 of 'intel_pin_and_fence_fb_obj'
> ret = intel_pin_and_fence_fb_obj(NULL, ifbdev->fb->base, NULL);
> ^
>In file included from drivers/gpu/drm/i915/intel_fbdev.c:44:0:
>drivers/gpu/drm/i915/intel_drv.h:1098:5: note: expected 'struct
> drm_framebuffer *' but argument is of type 'struct drm_framebuffer'
> int intel_pin_and_fence_fb_obj(struct drm_plane *plane,
With the missing & ack for -fixes on both patches from my side.
-Daniel
> ^
>
> vim +225 drivers/gpu/drm/i915/intel_fbdev.c
>
>209} else {
>210DRM_DEBUG_KMS("re-using BIOS fb\n");
>211prealloc = true;
>212sizes->fb_width = intel_fb->base.width;
>213sizes->fb_height = intel_fb->base.height;
>214}
>215
>216obj = intel_fb->obj;
>217size = obj->base.size;
>218
>219mutex_lock(&dev->struct_mutex);
>220
>221/* Pin the GGTT vma for our access via
> info->screen_base.
>222 * This also validates that any existing fb inherited
> from the
>223 * BIOS is suitable for own access.
>224 */
> > 225ret = intel_pin_and_fence_fb_obj(NULL,
> ifbdev->fb->base, NULL);
>226if (ret)
>227goto out_unlock;
>228
>229info = drm_fb_helper_alloc_fbi(helper);
>230if (IS_ERR(info)) {
>231DRM_ERROR("Failed to allocate fb_info\n");
>232ret = PTR_ERR(info);
>233goto out_unpin;
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: Check the timeout passed to i915_wait_request
On Thu, Nov 26, 2015 at 01:31:42PM +, Chris Wilson wrote:
> We have relied upon the sole caller (wait_ioctl) validating the timeout
> argument. However, when waiting for multiple requests I forgot to ensure
> that the timeout was still positive on the later requests. This is more
> simply done inside __i915_wait_request.
>
> Fixes regression introduced in
> commit b47161858ba13c9c7e0132230d66e008dd55
> Author: Chris Wilson
> Date: Mon Apr 27 13:41:17 2015 +0100
>
> drm/i915: Implement inter-engine read-read optimisations
>
> Signed-off-by: Chris Wilson
> Cc: Lionel Landwerlin
> Cc: Tvrtko Ursulin
> Cc: Daniel Vetter
> Cc: stable@vger.kernel.org
Commit message should explain what the actual problem is - we add 1 jiffy
of delay for each wait_request, potentially waiting quite a bit longer
than what userspace asked for.
And not sure this really justifies for cc: stable, since all the wait
syscalls reserve the right to wait longer. Of course we should fix it,
just to keep validating this possible.
So with the commit message amended and cc: stable dropped this is
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73c2c48729ec..8c19a980f5e6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1210,8 +1210,16 @@ int __i915_wait_request(struct drm_i915_gem_request
> *req,
> if (i915_gem_request_completed(req, true))
> return 0;
>
> - timeout_expire = timeout ?
> - jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
> + timeout_expire = 0;
> + if (timeout) {
> + if (WARN_ON(*timeout < 0))
> + return -EINVAL;
> +
> + if (*timeout == 0)
> + return -ETIME;
> +
> + timeout_expire = jiffies + nsecs_to_jiffies_timeout(*timeout);
> + }
>
> if (INTEL_INFO(dev_priv)->gen >= 6)
> gen6_rps_boost(dev_priv, rps, req->emitted_jiffies);
> --
> 2.6.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] drm: Don't overwrite UNVERFIED mode status to OK
On Thu, Dec 03, 2015 at 11:14:09PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The way the mode probing works is this:
> 1. All modes currently on the mode list are marked as UNVERIFIED
> 2. New modes are on the probed_modes list (they start with
>status OK)
> 3. Modes are moved from the probed_modes list to the actual
>mode list. If a mode already on the mode list is deemed
>to match one of the probed modes, the duplicate is dropped
>and the mode status updated to OK. After this the
>probed_modes list will be empty.
> 4. All modes on the mode list are verified to not violate any
>constraints. Any that do are marked as such.
> 5. Any mode left with a non-OK status is pruned from the list,
>with an appropriate debug message.
This would look really pretty as a kerneldoc addition to
probe_single_connector(). And with asciidoc we can even do pretty ordered
lists like these. Can you please follow-up with a patch for that?
>
> What all this means is that any mode on the original list that
> didn't have a duplicate on the probed_modes list, should be left
> with status UNVERFIED (or previously could have been left with
> some other status, but never OK).
>
> I broke that in
> commit 05acaec334fc ("drm: Reorganize probed mode validation")
> by always assigning something to the mode->status during the validation
> step. So any mode from the old list that still passed the validation
> would be left on the list with status OK in the end.
>
> Fix this by not doing the basic mode validation unless the mode
> already has status OK (meaning it came from the probed_modes list,
> or at least a duplicate of it was on that list). This way we will
> correctly prune away any mode from the old mode list that didn't
> appear on the probed_modes list.
>
> Cc: stable@vger.kernel.org
> Cc: Adam Jackson
> Fixes: 05acaec334fc ("drm: Reorganize probed mode validation")
> Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_probe_helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 94ba39e34299..b9b3bd9349ff 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -229,7 +229,8 @@ static int
> drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
> mode_flags |= DRM_MODE_FLAG_3D_MASK;
>
> list_for_each_entry(mode, &connector->modes, head) {
> - mode->status = drm_mode_validate_basic(mode);
> + if (mode->status == MODE_OK)
> + mode->status = drm_mode_validate_basic(mode);
>
> if (mode->status == MODE_OK)
> mode->status = drm_mode_validate_size(mode, maxX, maxY);
> --
> 2.4.10
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: Fix RPS pointer passed from wait_ioctl to i915_wait_request
On Thu, Dec 03, 2015 at 09:25:37PM +, Chris Wilson wrote: > On Wed, Dec 02, 2015 at 09:13:46AM +, Chris Wilson wrote: > > In commit 2e1b873072dfe3bbcc158a9c21acde1ab0d36c55 [v4.2] > > Author: Chris Wilson > > Date: Mon Apr 27 13:41:22 2015 +0100 > > > > drm/i915: Convert RPS tracking to a intel_rps_client struct > > > > we converted the __i915_wait_request() to take a new intel_rps_client > > struct (rather than having to pass fake drm_i915_file_private structs). > > However, due to use of passing a void pointer, I didn't spot one > > callsite in wait-ioctl was passing the wrong pointer. > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: stable@vger.kernel.org > > Fwiw, the impact of this bug is zero. Along the rps path, we always > first call list_empty(rps) which when we pass in the wrong pointer > always evaluates to false and we return early and never chase the > invalid pointers. > > The user visible impact is then wait-ioctl doesn't get the same > waitboosting as the other interfaces (set-domain, throttle), which is a > performance concern for the *very* few users of the wait interface. > There is also a libdrm_intel patch to use the wait-ioctl for > drm_intel_bo_wait_rendering() if anyone feels inclined to review > libdrm_intel patches. I added this to the commit message and figured it's not 100% justified for -fixes. So applied to dinq. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH v2] drm/i915: Do a better job at disabling primary plane in the noatomic case.
On Mon, Nov 23, 2015 at 10:25:28AM +0100, Maarten Lankhorst wrote: > Op 13-11-15 om 14:28 schreef Ander Conselvan De Oliveira: > > On Thu, 2015-11-12 at 14:58 +0100, Maarten Lankhorst wrote: > >> Op 12-11-15 om 14:37 schreef Ander Conselvan De Oliveira: > >>> On Wed, 2015-11-11 at 15:36 +0100, Maarten Lankhorst wrote: > >>>> When disable_noatomic is called plane_mask is not reliable yet, > >>>> and plane_state->visible = true even after disabling the primary plane. > >>> So the stale value of plane_state->visible causes a subsequent modeset to > >>> enable > >>> the primary again? > >> Probably not because it would get recalculated in calc_changes, but it > >> should > >> really be set to false afterwards. > > So basically I didn't understand how the wrong value of plane_state->visible > > causes the bug that was mentioned. I think a brief explanation in the commit > > message would be good. > Well, apply with git am --scissors. Same patch but mentioning this. > > Is this better? > >8- > > When disable_noatomic is called plane_mask is not correct yet, > and plane_state->visible = true is left as true after disabling > the primary plane. > > Other planes are already disabled as part of crtc sanitization, only the > primary is left active. But the plane_mask is not updated here. It gets > updated > during fb takeover in modeset_gem_init, or set to the new value on resume. > > This means that to disable the primary plane 1 << drm_plane_index(primary) > needs to be used. > > Afterwards because the crtc is no longer active it's forbidden to keep > plane_state->visible set, or a WARN_ON in intel_plane_atomic_calc_changes > triggers. There are other code points that rely on accurate > plane_state->visible > too, so make sure the bool is cleared. Oh dear is this a mess :( As a bugfix this is Reviewed-by: Daniel Vetter But we need to bring a _lot_ more sanity to this in -next. Would probably be better if we keep the primary plane state all consistent, and then clear both visible and plane_mask when fb reconstruction fails. Also, some WARN_ON to bitch about inconsistency between plane_mask and ->visible in the disable code paths would be good I think. -Daniel > > > The other planes are already disabled in intel_sanitize_crtc, so > they don't have to be handled here. > > Cc: stable@vger.kernel.org #v4.3, v4.2? > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92655 > Tested-by: Tomas Mezzadra > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_display.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index b5f7493213b7..bc3282ab5ed2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6267,9 +6267,11 @@ static void intel_crtc_disable_noatomic(struct > drm_crtc *crtc) > WARN_ON(intel_crtc->unpin_work); > > intel_pre_disable_primary(crtc); > + > + intel_crtc_disable_planes(crtc, 1 << > drm_plane_index(crtc->primary)); > + to_intel_plane_state(crtc->primary->state)->visible = false; > } > > - intel_crtc_disable_planes(crtc, crtc->state->plane_mask); > dev_priv->display.crtc_disable(crtc); > intel_crtc->active = false; > intel_update_watermarks(crtc); > -- 2.1.0 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Unbreak check_digital_port_conflicts()
On Mon, Dec 07, 2015 at 11:13:20PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Atomic changes broke check_digital_port_conflicts(). It needs to look
> at the global situation instead of just trying to find a conflict
> within the current atomic state.
>
> This bug made my HSW explode spectacularly after I had split the DDI
> encoders into separate DP and HDMI encoders. With the fix, things
> seem much more solid.
>
> I hope holding the connection_mutex is enough protection that we can
> actually walk the connectors even if they're not part of the current
> atomic state...
That is sufficient locking.
>
> Cc: stable@vger.kernel.org
Ugh. Long-term I think what we need is for all drivers (well at least
atomic ones) to fill out the possible_clones stuff correctly in the
encoder. And then check this in the atomic helpers. But that's way too
much for -fixes.
On the patch itself, for -fixes: Reviewed-by: Daniel Vetter
> Cc: Ander Conselvan de Oliveira
> Fixes: 5448a00d3f06 ("drm/i915: Don't use staged config in
> check_digital_port_conflicts()")
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1c6d56c84b9d..c902964ceca0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12253,18 +12253,23 @@ static void intel_dump_pipe_config(struct
> intel_crtc *crtc,
>
> static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> {
> - struct intel_encoder *encoder;
> + struct drm_device *dev = state->dev;
> struct drm_connector *connector;
> - struct drm_connector_state *connector_state;
> unsigned int used_ports = 0;
> - int i;
>
> /*
>* Walk the connector list instead of the encoder
>* list to detect the problem on ddi platforms
>* where there's just one encoder per digital port.
>*/
> - for_each_connector_in_state(state, connector, connector_state, i) {
> + drm_for_each_connector(connector, dev) {
> + struct drm_connector_state *connector_state;
> + struct intel_encoder *encoder;
> +
> + connector_state =
> drm_atomic_get_existing_connector_state(state, connector);
> + if (!connector_state)
> + connector_state = connector->state;
> +
> if (!connector_state->best_encoder)
> continue;
>
> --
> 2.4.10
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping
On Sun, Dec 06, 2015 at 09:33:20PM +0100, Lukas Wunner wrote:
> Hi Chris,
>
> On Fri, Dec 04, 2015 at 04:05:26PM +, Chris Wilson wrote:
> > A long time ago (before 3.14) we relied on a permanent pinning of the
> > ifbdev to lock the fb in place inside the GGTT. However, the
> > introduction of stealing the BIOS framebuffer and reusing its address in
> > the GGTT for the fbdev has muddied waters and we use an inherited fb.
> > However, the inherited fb is only pinned whilst it is active and we no
> > longer have an explicit pin for the info->system_base mmapping used by
> > the fbdev. The result is that after some aperture pressure the fbdev may
> > be evicted, but we continue to write the fbcon into the same GGTT
> > address - overwriting anything else that may be put into that offset.
> > The effect is most pronounced across suspend/resume as
> > intel_fbdev_set_suspend() does a full clear over the whole scanout.
> >
> > v2: Only unpin the intel_fb is we allocate it. If we inherit the fb from
> > the BIOS, we do not own the pinned vma (except for the reference we add
> > in this patch for our access via info->screen_base).
> >
> > v3: Finish balancing the vma pinning for the normal !preallocated case.
> >
> > v4: Try to simplify the pinning even further.
> > v5: Leak the VMA (cleaned up by object-free) to avoid complicated error
> > paths.
>
> It's beautiful how little code is needed to fix this. The only remaining
> thing I noticed now while looking over the error paths is that these
> lines in intelfb_alloc() become obsolete with your patch:
>
> out:
> mutex_unlock(&dev->struct_mutex);
> - if (!IS_ERR_OR_NULL(fb))
> - drm_framebuffer_unreference(fb);
> return ret;
> }
>
> Because at each of the remaining "goto out" in the function,
> fb can be only either an ERR_PTR or NULL.
>
> Also, further up in the function, the declaration of fb can then be
> changed thus:
>
> - struct drm_framebuffer *fb = NULL;
> + struct drm_framebuffer *fb;
>
> Kind regards,
Yeah there's room for follow-up polish, but this seems good enough at
least for -fixes.
Reviewed-by: Daniel Vetter
Lukas, feel like supplying a patch to apply the polish you've spotted on
top?
Thanks, Daniel
>
> Lukas
>
> >
> > Signed-off-by: Chris Wilson
> > Cc: "Goel, Akash"
> > Cc: Daniel Vetter
> > Cc: Jesse Barnes
> > Cc: Lukas Wunner
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/gpu/drm/i915/intel_fbdev.c | 20 +---
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 7ccde58f8c98..bea75cafc623 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -163,13 +163,6 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> > goto out;
> > }
> >
> > - /* Flush everything out, we'll be doing GTT only from now on */
> > - ret = intel_pin_and_fence_fb_obj(NULL, fb, NULL);
> > - if (ret) {
> > - DRM_ERROR("failed to pin obj: %d\n", ret);
> > - goto out;
> > - }
> > -
> > mutex_unlock(&dev->struct_mutex);
> >
> > ifbdev->fb = to_intel_framebuffer(fb);
> > @@ -225,6 +218,14 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >
> > mutex_lock(&dev->struct_mutex);
> >
> > + /* Pin the GGTT vma for our access via info->screen_base.
> > +* This also validates that any existing fb inherited from the
> > +* BIOS is suitable for own access.
> > +*/
> > + ret = intel_pin_and_fence_fb_obj(NULL, &ifbdev->fb->base, NULL);
> > + if (ret)
> > + goto out_unlock;
> > +
> > info = drm_fb_helper_alloc_fbi(helper);
> > if (IS_ERR(info)) {
> > DRM_ERROR("Failed to allocate fb_info\n");
> > @@ -287,6 +288,7 @@ out_destroy_fbi:
> > drm_fb_helper_release_fbi(helper);
> > out_unpin:
> > i915_gem_object_ggtt_unpin(obj);
> > +out_unlock:
> > mutex_unlock(&dev->struct_mutex);
> > return ret;
> > }
> > @@ -524,6 +526,10 @@ static const struct drm_fb_helper_funcs
> > intel_fb_helper_funcs = {
> > static void intel_fbdev_destroy(struct drm_device *dev,
> > struct intel_fbdev *ifbdev)
> > {
> > + /* We rely on the object-free to release the VMA pinning for
> > +* the info->screen_base mmaping. Leaking the VMA is simpler than
> > +* trying to rectify all the possible error paths leading here.
> > +*/
> >
> > drm_fb_helper_unregister_fbi(&ifbdev->helper);
> > drm_fb_helper_release_fbi(&ifbdev->helper);
> > --
> > 2.6.2
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH v5] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping
On Thu, Dec 10, 2015 at 05:41:30PM +0100, Takashi Iwai wrote: > On Thu, 10 Dec 2015 17:36:04 +0100, > Ville Syrjälä wrote: > > > > On Fri, Dec 04, 2015 at 04:05:26PM +, Chris Wilson wrote: > > > A long time ago (before 3.14) we relied on a permanent pinning of the > > > ifbdev to lock the fb in place inside the GGTT. However, the > > > introduction of stealing the BIOS framebuffer and reusing its address in > > > the GGTT for the fbdev has muddied waters and we use an inherited fb. > > > However, the inherited fb is only pinned whilst it is active and we no > > > longer have an explicit pin for the info->system_base mmapping used by > > > the fbdev. The result is that after some aperture pressure the fbdev may > > > be evicted, but we continue to write the fbcon into the same GGTT > > > address - overwriting anything else that may be put into that offset. > > > The effect is most pronounced across suspend/resume as > > > intel_fbdev_set_suspend() does a full clear over the whole scanout. > > > > > > v2: Only unpin the intel_fb is we allocate it. If we inherit the fb from > > > the BIOS, we do not own the pinned vma (except for the reference we add > > > in this patch for our access via info->screen_base). > > > > > > v3: Finish balancing the vma pinning for the normal !preallocated case. > > > > > > v4: Try to simplify the pinning even further. > > > v5: Leak the VMA (cleaned up by object-free) to avoid complicated error > > > paths. > > > > > > Signed-off-by: Chris Wilson > > > Cc: "Goel, Akash" > > > Cc: Daniel Vetter > > > Cc: Jesse Barnes > > > Cc: Lukas Wunner > > > Cc: stable@vger.kernel.org > > > > This seems to have fixed my garbled text+fbcon dead after > > suspend/hibernate issues. Well, only had the patch in for a day or so, > > but so far so good. > > > > Tested-by: Ville Syrjälä > > > > Takashi, don't know if you already found this patch, but it's definitely > > something you should try as well. > > Great, I'll give this a try. Thanks! Pulled both patches into dinq. Jani, can you please cherry-pick? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/32] drm/i915: Only spin whilst waiting on the current request
On Fri, Dec 11, 2015 at 11:32:59AM +, Chris Wilson wrote:
> Limit busywaiting only to the request currently being processed by the
> GPU. If the request is not currently being processed by the GPU, there
> is a very low likelihood of it being completed within the 2 microsecond
> spin timeout and so we will just be wasting CPU cycles.
>
> v2: Check for logical inversion when rebasing - we were incorrectly
> checking for this request being active, and instead busywaiting for
> when the GPU was not yet processing the request of interest.
>
> v3: Try another colour for the seqno names.
> v4: Another colour for the function names.
>
> v5: Remove the forced coherency when checking for the active request. On
> reflection and plenty of recent experimentation, the issue is not a
> cache coherency problem - but an irq/seqno ordering problem (timing issue).
> Here, we do not need the w/a to force ordering of the read with an
> interrupt.
>
> Signed-off-by: Chris Wilson
> Reviewed-by: Tvrtko Ursulin
> Cc: "Rogozhkin, Dmitry V"
> Cc: Daniel Vetter
> Cc: Tvrtko Ursulin
> Cc: Eero Tamminen
> Cc: "Rantala, Valtteri"
> Cc: stable@vger.kernel.org
Merged these 3 patches, thanks.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 27 +++
> drivers/gpu/drm/i915/i915_gem.c | 8 +++-
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5edd39352e97..8c4303b664d9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2182,8 +2182,17 @@ struct drm_i915_gem_request {
> struct drm_i915_private *i915;
> struct intel_engine_cs *ring;
>
> - /** GEM sequence number associated with this request. */
> - uint32_t seqno;
> + /** GEM sequence number associated with the previous request,
> + * when the HWS breadcrumb is equal to this the GPU is processing
> + * this request.
> + */
> + u32 previous_seqno;
> +
> + /** GEM sequence number associated with this request,
> + * when the HWS breadcrumb is equal or greater than this the GPU
> + * has finished processing this request.
> + */
> + u32 seqno;
>
> /** Position in the ringbuffer of the start of the request */
> u32 head;
> @@ -2958,15 +2967,17 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
> return (int32_t)(seq1 - seq2) >= 0;
> }
>
> +static inline bool i915_gem_request_started(struct drm_i915_gem_request *req,
> +bool lazy_coherency)
> +{
> + u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency);
> + return i915_seqno_passed(seqno, req->previous_seqno);
> +}
> +
> static inline bool i915_gem_request_completed(struct drm_i915_gem_request
> *req,
> bool lazy_coherency)
> {
> - u32 seqno;
> -
> - BUG_ON(req == NULL);
> -
> - seqno = req->ring->get_seqno(req->ring, lazy_coherency);
> -
> + u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency);
> return i915_seqno_passed(seqno, req->seqno);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 46a84c447d8f..29d98ddbbc80 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1193,9 +1193,13 @@ static int __i915_spin_request(struct
> drm_i915_gem_request *req, int state)
>* takes to sleep on a request, on the order of a microsecond.
>*/
>
> - if (i915_gem_request_get_ring(req)->irq_refcount)
> + if (req->ring->irq_refcount)
> return -EBUSY;
>
> + /* Only spin if we know the GPU is processing this request */
> + if (!i915_gem_request_started(req, true))
> + return -EAGAIN;
> +
> timeout = local_clock_us(&cpu) + 5;
> while (!need_resched()) {
> if (i915_gem_request_completed(req, true))
> @@ -1209,6 +1213,7 @@ static int __i915_spin_request(struct
> drm_i915_gem_request *req, int state)
>
> cpu_relax_lowlatency();
> }
> +
> if (i915_gem_request_completed(req, false))
> return 0;
>
> @@ -2600,6 +2605,7 @@ void __i915_add_request(struct drm_i915_gem_request
> *request,
> request->batch_obj = obj;
>
> request->emitted_jiffies = jiffies;
> + request->previous_seqno = ring->last_submitted_seqno;
> ring->last_submitted_seqno = request->seqno;
> list_add_tail(&request->list, &ring->request_list);
>
> --
> 2.6.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: Hide one invalid cancellation bug in i915_switch_context()
On Thu, Dec 17, 2015 at 06:18:05PM +, Chris Wilson wrote:
> As we add the VMA to the request early, it may be cancelled during
> execbuf reservation. This will leave the context object pointing to a
> dangling request; i915_wait_request() simply skips the wait and so we
> may unbind the object whilst it is still active.
>
> We can partially prevent such atrocity by doing the RCS context
> initialisation earlier. This ensures that one callsite from blowing up
> (and for igt this is a frequent culprit due to how the stressful batches
> are submitted).
>
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 29 +
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 900ffd044db8..657686e6492f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -657,7 +657,6 @@ static int do_switch(struct drm_i915_gem_request *req)
> struct drm_i915_private *dev_priv = ring->dev->dev_private;
> struct intel_context *from = ring->last_context;
> u32 hw_flags = 0;
> - bool uninitialized = false;
> int ret, i;
>
> if (from != NULL && ring == &dev_priv->ring[RCS]) {
> @@ -764,6 +763,15 @@ static int do_switch(struct drm_i915_gem_request *req)
> to->remap_slice &= ~(1< }
>
> + if (!to->legacy_hw_ctx.initialized) {
> + if (ring->init_context) {
> + ret = ring->init_context(req);
> + if (ret)
> + goto unpin_out;
> + }
> + to->legacy_hw_ctx.initialized = true;
> + }
> +
> /* The backing object for the context is done after switching to the
>* *next* context. Therefore we cannot retire the previous context until
>* the next context has already started running. In fact, the below code
> @@ -772,6 +780,14 @@ static int do_switch(struct drm_i915_gem_request *req)
>*/
> if (from != NULL) {
> from->legacy_hw_ctx.rcs_state->base.read_domains =
> I915_GEM_DOMAIN_INSTRUCTION;
> + /* XXX Note very well this is dangerous!
> + * We are pinning this object using this request as our
> + * active reference. However, this request may yet be cancelled
> + * during the execbuf dispatch, leaving us waiting on a
> + * dangling request. Waiting upon this dangling request is
> + * ignored, which means that we may unbind the context whilst
> + * the GPU is still writing to the backing storage.
> + */
>
> i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state),
> req);
Hm, since this is just a partial fix, what about instead marking any
request that has been put to use already in move_to_active. And then when
we try to cancel it from execbuf notice that and not cancel it? Fixes both
bugs and makes the entire thing a bit more robust since it allows us to
throw stuff at a request without ordering constraints.
-Daniel
> /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
>* whole damn pipeline, we don't need to explicitly mark the
> @@ -787,21 +803,10 @@ static int do_switch(struct drm_i915_gem_request *req)
> i915_gem_context_unreference(from);
> }
>
> - uninitialized = !to->legacy_hw_ctx.initialized;
> - to->legacy_hw_ctx.initialized = true;
> -
> done:
> i915_gem_context_reference(to);
> ring->last_context = to;
>
> - if (uninitialized) {
> - if (ring->init_context) {
> - ret = ring->init_context(req);
> - if (ret)
> - DRM_ERROR("ring init context: %d\n", ret);
> - }
> - }
> -
> return 0;
>
> unpin_out:
> --
> 2.6.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Workaround CHV pipe C cursor fail
On Fri, Dec 18, 2015 at 07:24:39PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Turns out CHV pipe C was glued on somewhat poorly, and there's something
> wrong with the cursor. If the cursor straddles the left screen edge,
> and is then moved away from the edge or disabled, the pipe will often
> underrun. If enough underruns are triggered quickly enough the pipe
> will fall over and die (it just scans out a solid color and reports
> a constant underrun). We need to turn the disp2d power well off and
> on again to recover the pipe.
>
> None of that is very nice for the user, so let's just refuse to place
> the cursor in the compromised position. The ddx appears to fall back
> to swcursor when the ioctl returns an error, so theoretically there's
> no loss of functionality for the user (discounting swcursor bugs).
> I suppose most cursors images actually have the hotspot not exactly
> at 0,0 so under typical conditions the fallback will in fact kick in
> as soon as the cursor touches the left edge of the screen.
>
> Any atomic compositor should anyway be prepared to fall back to
> GPU composition when things don't work out, so there should be no
> problem with those.
>
> Other things that I tried to solve this include flipping all
> display related clock gating knobs I could find, increasing the
> minimum gtt alignment all the way up to 512k. I also tried to see
> if there are more specific screen coordinates that hit the bug, but
> the findings were somewhat inconclusive. Sometimes the failures
> happen almost across the whole left edge, sometimes more at the very
> top and around the bottom half. I wasn't able to find any real pattern
> to these variations, so it seems our only choice is to just refuse
> to straddle the left screen edge at all.
>
> Cc: stable@vger.kernel.org
> Cc: Jason Plum
> Testcase: igt/kms_chv_cursor_fail
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92826
> Signed-off-by: Ville Syrjälä
Well can't really review with Bspec and I don't expect we can get
confirmation from hw engineers. But does what it says in the commit
message.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index abd2d2944022..8acc66b95139 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14059,6 +14059,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
> struct drm_crtc *crtc = crtc_state->base.crtc;
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + enum pipe pipe = to_intel_plane(plane)->pipe;
> unsigned stride;
> int ret;
>
> @@ -14092,6 +14093,22 @@ intel_check_cursor_plane(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + /*
> + * There's something wrong with the cursor on CHV pipe C.
> + * If it straddles the left edge of the screen then
> + * moving it away from the edge or disabling it often
> + * results in a pipe underrun, and often that can lead to
> + * dead pipe (constant underrun reported, and it scans
> + * out just a solid color). To recover from that, the
> + * display power well must be turned off and on again.
> + * Refuse the put the cursor into that compromised position.
> + */
> + if (IS_CHERRYVIEW(plane->dev) && pipe == PIPE_C &&
> + state->visible && state->base.crtc_x < 0) {
> + DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left
> screen edge\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> --
> 2.4.10
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Workaround CHV pipe C cursor fail
On Mon, Dec 21, 2015 at 02:49:18PM +0100, Daniel Vetter wrote:
> On Fri, Dec 18, 2015 at 07:24:39PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Turns out CHV pipe C was glued on somewhat poorly, and there's something
> > wrong with the cursor. If the cursor straddles the left screen edge,
> > and is then moved away from the edge or disabled, the pipe will often
> > underrun. If enough underruns are triggered quickly enough the pipe
> > will fall over and die (it just scans out a solid color and reports
> > a constant underrun). We need to turn the disp2d power well off and
> > on again to recover the pipe.
> >
> > None of that is very nice for the user, so let's just refuse to place
> > the cursor in the compromised position. The ddx appears to fall back
> > to swcursor when the ioctl returns an error, so theoretically there's
> > no loss of functionality for the user (discounting swcursor bugs).
> > I suppose most cursors images actually have the hotspot not exactly
> > at 0,0 so under typical conditions the fallback will in fact kick in
> > as soon as the cursor touches the left edge of the screen.
> >
> > Any atomic compositor should anyway be prepared to fall back to
> > GPU composition when things don't work out, so there should be no
> > problem with those.
> >
> > Other things that I tried to solve this include flipping all
> > display related clock gating knobs I could find, increasing the
> > minimum gtt alignment all the way up to 512k. I also tried to see
> > if there are more specific screen coordinates that hit the bug, but
> > the findings were somewhat inconclusive. Sometimes the failures
> > happen almost across the whole left edge, sometimes more at the very
> > top and around the bottom half. I wasn't able to find any real pattern
> > to these variations, so it seems our only choice is to just refuse
> > to straddle the left screen edge at all.
> >
> > Cc: stable@vger.kernel.org
> > Cc: Jason Plum
> > Testcase: igt/kms_chv_cursor_fail
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92826
> > Signed-off-by: Ville Syrjälä
>
> Well can't really review with Bspec and I don't expect we can get
> confirmation from hw engineers. But does what it says in the commit
> message.
>
> Reviewed-by: Daniel Vetter
And applied to dinq since that's our new flow even for fixes.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 17 +
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index abd2d2944022..8acc66b95139 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14059,6 +14059,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > struct drm_crtc *crtc = crtc_state->base.crtc;
> > struct drm_framebuffer *fb = state->base.fb;
> > struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> > + enum pipe pipe = to_intel_plane(plane)->pipe;
> > unsigned stride;
> > int ret;
> >
> > @@ -14092,6 +14093,22 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > return -EINVAL;
> > }
> >
> > + /*
> > +* There's something wrong with the cursor on CHV pipe C.
> > +* If it straddles the left edge of the screen then
> > +* moving it away from the edge or disabling it often
> > +* results in a pipe underrun, and often that can lead to
> > +* dead pipe (constant underrun reported, and it scans
> > +* out just a solid color). To recover from that, the
> > +* display power well must be turned off and on again.
> > +* Refuse the put the cursor into that compromised position.
> > +*/
> > + if (IS_CHERRYVIEW(plane->dev) && pipe == PIPE_C &&
> > + state->visible && state->base.crtc_x < 0) {
> > + DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left
> > screen edge\n");
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.4.10
> >
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH v2] drm/i915: intel_hpd_init(): Fix suspend/resume reprobing
On Tue, Jan 05, 2016 at 04:15:30PM -0500, Lyude wrote:
> This fixes reprobing of display connectors on resume. After some talking
> with danvet on IRC, I learned that calling drm_helper_hpd_irq_event() does
> actually trigger a full reprobe of each connector's status. It turns out
> this is the actual reason reprobing on resume hasn't been working (this was
> observed on a T440s):
>
> - We call hpd_init()
> - We check each connector for a couple of things before marking
> connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> active encoder. Of course, a disconnected port won't have an
> active encoder, so we don't add the flag to any of the
> connectors.
> - We call drm_helper_hpd_irq_event()
> - drm_helper_irq_event() checks each connector for the
> DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
> so we skip reprobing each connector except for that one.
>
> In addition, we now avoid setting connector->polled to
> DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is handled
> by the mst helpers. This is probably what was originally intended to happen
> here anyway.
>
> Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lyude
> ---
> Changes
> * Use the explanation of the issue as the commit message instead
> * Change the title of the commit, since this does more then just stop a check
> for an encoder now
> * Add "Fixes" line for the patch that introduced this regression
> * Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
For drm/i915 I prefer when the patch changelog is above the s-o-b section,
with v2: v3: ... headings.
>
> drivers/gpu/drm/i915/intel_hotplug.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..51ecf0b4 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -468,9 +468,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> list_for_each_entry(connector, &mode_config->connector_list, head) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
> connector->polled = intel_connector->polled;
> - if (connector->encoder && !connector->polled &&
> I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> - if (intel_connector->mst_port)
> + if (!connector->polled && !intel_connector->mst_port &&
> + I915_HAS_HOTPLUG(dev) &&
> + intel_connector->encoder->hpd_pin > HPD_NONE)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
Hm, on 2nd thought we could clarify this further like this:
list_for_each_entry(...) {
...
/* MST has a dynamic intel_connector->encoder and it's
* reprobing is all handled by the MST helpers. */
if (intel_connector->mst_port)
continue;
if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
intel_connector->encoder->hpd_pin > HPD_NONE)
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
That yields even tidier code and gives us a place for a comment. Can you
pls respin once more?
Thanks, Daniel
> }
>
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b4741d121a74..405aba2ca736 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv);
-
ret = intel_irq_install(dev_priv);
if (ret)
goto cleanup_gem_stolen;
@@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Tested-by: Meelis Roos
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..490d8b0d931e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
- goto out_power_well;
+ goto out_vblank;
}
/*
@@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank:
drm_vblank_cleanup(dev);
out_gem_unload:
WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
@@ -1103,6 +1101,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
+ intel_power_domains_fini(dev_priv);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: Init power domains early in driver load
On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrjälä
wrote:
> On Thu, Jan 07, 2016 at 12:44:21PM +0100, Daniel Vetter wrote:
>> Since
>>
>> commit ac9b8236551d1177fd07b56aef9b565d1864420d
>> Author: Ville Syrjälä
>> Date: Fri Nov 27 18:55:26 2015 +0200
>>
>> drm/i915: Introduce a gmbus power domain
>>
>> gmbus also needs the power domain infrastructure right from the start,
>> since as soon as we register the i2c controllers someone can use them.
>>
>> v2: Adjust cleanup paths too (Chris).
>>
>> Cc: Ville Syrjälä
>> Cc: Patrik Jakobsson
>> Cc: Imre Deak
>> Cc: Jani Nikula
>> Cc: Meelis Roos
>> Cc: Chris Wilson
>> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
>> Cc: stable@vger.kernel.org
>> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
>> Tested-by: Meelis Roos
>> Signed-off-by: Daniel Vetter
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 11 +--
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c
>> b/drivers/gpu/drm/i915/i915_dma.c
>> index 988a3806512a..490d8b0d931e 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
>> if (ret)
>> goto cleanup_vga_switcheroo;
>>
>> - intel_power_domains_init_hw(dev_priv, false);
>>
>> intel_csr_ucode_init(dev_priv);
>>
>> @@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
>> long flags)
>>
>> intel_irq_init(dev_priv);
>> intel_uncore_sanitize(dev);
>> + intel_power_domains_init(dev_priv);
>> + intel_power_domains_init_hw(dev_priv);
>
> I think intel_init_dpio() needs to be moved too. We need to know the
> DPIO IOSF ports before attempting to talk to the PHY (which can happen
> from intel_power_domains_init_hw()).
Ugh, will change.
> I'm also wondering why we're doing gmbus init this early. We shouldn't
> need it until modeset init.
Anyone can access the gmbus controller once we register it. Userspace
can (like what seems to happen on Meelis' box), but also the i2c core
has some auto-probed stuff in some configs afaik.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Tested-by: Meelis Roos
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..fcefd3beef50 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,9 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_init_dpio(dev_priv);
+ intel_power_domains_init_hw(dev_priv, false);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1049,20 +1051,16 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_device_info_runtime_init(dev);
- intel_init_dpio(dev_priv);
-
if (INTEL_INFO(dev)->num_pipes) {
ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
if (ret)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
- goto out_power_well;
+ goto out_vblank;
}
/*
@@ -1091,8 +1089,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank:
drm_vblank_cleanup(dev);
out_gem_unload:
WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
@@ -1103,6 +1100,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
+ intel_power_domains_fini(dev_priv);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).
v4: Ville instead suggested to move gmbus setup later in the sequence,
since it's only needed by the modeset code.
v5: Move even close to the actual user, right next to the comment that
states where we really need gmbus (and interrupts!).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: stable@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter
---
Meelis, can you pls retest this one?
Thanks, Daniel
---
drivers/gpu/drm/i915/i915_dma.c | 6 +++---
drivers/gpu/drm/i915/intel_display.c | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..d70d96fe553b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_gem_stolen;
+ intel_setup_gmbus(dev);
+
/* Important: The output setup functions called by modeset_init need
* working irqs for e.g. gmbus and dp aux transfers. */
intel_modeset_init(dev);
@@ -455,6 +457,7 @@ cleanup_gem:
cleanup_irq:
intel_guc_ucode_fini(dev);
drm_irq_uninstall(dev);
+ intel_teardown_gmbus(dev);
cleanup_gem_stolen:
i915_gem_cleanup_stolen(dev);
cleanup_vga_switcheroo:
@@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
- intel_setup_gmbus(dev);
intel_opregion_setup(dev);
i915_gem_load(dev);
@@ -1101,7 +1103,6 @@ out_gem_unload:
if (dev->pdev->msi_enabled)
pci_disable_msi(dev->pdev);
- intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
@@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev)
intel_csr_ucode_fini(dev_priv);
- intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv->hotplug.dp_wq);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 37945ddb4dad..ac0038bf4fbf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
mutex_lock(&dev->struct_mutex);
intel_cleanup_gt_powersave(dev);
mutex_unlock(&dev->struct_mutex);
+
+ intel_teardown_gmbus(dev);
}
/*
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH v3] drm/i915: intel_hpd_init(): Fix suspend/resume reprobing
On Thu, Jan 07, 2016 at 10:43:28AM -0500, Lyude wrote:
> This fixes reprobing of display connectors on resume. After some
> talking with danvet on IRC, I learned that calling
> drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
> connector's status. It turns out this is the actual reason reprobing on
> resume hasn't been working (this was observed on a T440s):
>
> - We call hpd_init()
> - We check each connector for a couple of things before marking
> connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
> active encoder. Of course, a disconnected port won't have an
> active encoder, so we don't add the flag to any of the
> connectors.
> - We call drm_helper_hpd_irq_event()
> - drm_helper_irq_event() checks each connector for the
> DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
> so we skip reprobing each connector except that one.
>
> In addition, we also now avoid setting connector->polled to
> DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is
> handled by the mst helpers. This is probably what was originally
> intended to happen here.
>
> Changes since V1:
> * Use the explanation of the issue as the commit message instead
> * Change the title of the commit, since this does more then just stop a
> check for an encoder now
> * Add "Fixes" line for the patch that introduced this regression
> * Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
>
> Changes since V2:
> * Put patch changelog above Signed-off-by
> * Follow Daniel Vetter's suggestion for making the code here a bit more
> legible
>
> Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lyude
Awesome, and merged to drm-intel.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..d7a6437 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -468,9 +468,14 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> list_for_each_entry(connector, &mode_config->connector_list, head) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
> connector->polled = intel_connector->polled;
> - if (connector->encoder && !connector->polled &&
> I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
> - connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + /* MST has a dynamic intel_connector->encoder and it's reprobing
> + * is all handled by the MST helpers. */
> if (intel_connector->mst_port)
> + continue;
> +
> + if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
> + intel_connector->encoder->hpd_pin > HPD_NONE)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> }
>
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/kms-helper: disable hpd_irq handling when poll=0
When inlining the actual hpd output probing in commit 69787f7da6b2adc4054357a661aaa1701a9ca76f Author: Daniel Vetter Date: Tue Oct 23 18:23:34 2012 + drm: run the hpd irq event code directly the check for the drm_kms_hlper.poll module option was lost. This regressed systems where this option is used to work-around output probing issues (like irq storms). Restore the old behaviour. Reported-by: George Amanakis Cc: George Amanakis Cc: Dave Airlie Cc: stable@vger.kernel.org (for 3.8) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 7b2d378..3260736 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1067,7 +1067,7 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) enum drm_connector_status old_status; bool changed = false; - if (!dev->mode_config.poll_enabled) + if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll) return; mutex_lock(&dev->mode_config.mutex); -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux 3.8.6
Maintainer reporting in. But can we please drag the context in again? -Daniel On Sat, Apr 6, 2013 at 8:44 PM, Lucas wrote: > Alright, CC'ing the mantainers as well. > > Thanks again, > Lucas > > On 06/04/13 15:18, Greg KH wrote: >> >> >> Sure, can you cc: the authors of those patches, and anyone who >> signed-off on them in order to get their approval as well? >> >> thanks, >> >> greg k-h >> > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux 3.8.6
On Sun, Apr 7, 2013 at 12:56 AM, Lucas wrote: > Thanks for reporting in. > > Somehow the thread got all messed up in LKML. It didn't parse correctly > there, but it seems that Gmane did. Here is the link: > > http://comments.gmane.org/gmane.linux.kernel.stable/48872 Those patches still have an unresolved regression against them. Unfortunately I can't reproduce it despite that I have a very similar system, and obviously a bunch of other people like you reported that the exact same patches which regressed actually fix the lvds panel fitter. So for now this stays in limbo :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
On Fri, Apr 05, 2013 at 09:03:27AM -0700, Jesse Barnes wrote:
> On Thu, 4 Apr 2013 21:31:03 +0100
> Chris Wilson wrote:
>
> > In order to fully serialize access to the fenced region and the update
> > to the fence register we need to take extreme measures on SNB+, and
> > manually flush writes to memory prior to writing the fence register in
> > conjunction with the memory barriers placed around the register write.
> >
> > Fixes i-g-t/gem_fence_thrash
> >
> > v2: Bring a bigger gun
> > v3: Switch the bigger gun for heavier bullets (Arjan van de Ven)
> > v4: Remove changes for working generations.
> > v5: Reduce to a per-cpu wbinvd() call prior to updating the fences.
> > v6: Rewrite comments to ellide forgotten history.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
> > Signed-off-by: Chris Wilson
> > Cc: Jon Bloomfield
> > Tested-by: Jon Bloomfield (v2)
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 28 +++-
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index fa4ea1a..8f7739e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2689,17 +2689,35 @@ static inline int fence_number(struct
> > drm_i915_private *dev_priv,
> > return fence - dev_priv->fence_regs;
> > }
> >
> > +static void i915_gem_write_fence__ipi(void *data)
> > +{
> > + wbinvd();
> > +}
> > +
> > static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
> > struct drm_i915_fence_reg *fence,
> > bool enable)
> > {
> > - struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> > - int reg = fence_number(dev_priv, fence);
> > -
> > - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL);
> > + struct drm_device *dev = obj->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + int fence_reg = fence_number(dev_priv, fence);
> > +
> > + /* In order to fully serialize access to the fenced region and
> > +* the update to the fence register we need to take extreme
> > +* measures on SNB+. In theory, the write to the fence register
> > +* flushes all memory transactions before, and coupled with the
> > +* mb() placed around the register write we serialise all memory
> > +* operations with respect to the changes in the tiler. Yet, on
> > +* SNB+ we need to take a step further and emit an explicit wbinvd()
> > +* on each processor in order to manually flush all memory
> > +* transactions before updating the fence register.
> > +*/
> > + if (HAS_LLC(obj->base.dev))
> > + on_each_cpu(i915_gem_write_fence__ipi, NULL, 1);
> > + i915_gem_write_fence(dev, fence_reg, enable ? obj : NULL);
> >
> > if (enable) {
> > - obj->fence_reg = reg;
> > + obj->fence_reg = fence_reg;
> > fence->obj = obj;
> > list_move_tail(&fence->lru_list, &dev_priv->mm.fence_list);
> > } else {
>
> I almost wish x86 just gave up and went fully weakly ordered. At least
> then we'd know we need barriers everywhere, rather than just in
> mystical places.
>
> Reviewed-by: Jesse Barnes
Queued for -next, thanks for the patch. I am a bit unhappy that the
testcase doesn't work too well and can't reliably reproduce this on all
affected machines. But I've tried a bit to improve things and it's very
fickle. I guess we just have to accept this :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/kms-helper: disable hpd_irq handling when poll=0
On Mon, Apr 8, 2013 at 2:28 PM, Alex Deucher wrote: > On Sat, Apr 6, 2013 at 12:05 PM, Daniel Vetter wrote: >> When inlining the actual hpd output probing in >> >> commit 69787f7da6b2adc4054357a661aaa1701a9ca76f >> Author: Daniel Vetter >> Date: Tue Oct 23 18:23:34 2012 + >> >> drm: run the hpd irq event code directly >> >> the check for the drm_kms_hlper.poll module option was lost. This >> regressed systems where this option is used to work-around output >> probing issues (like irq storms). Restore the old behaviour. > > NAK. It's been a while since I looked at it, but the whole point of > this patch set was to be able to disabling polling independently of > hpd. If you add this check back, setting poll=0 disables both polling > and hpd. I'd suggest we add a separate hpd option to disable hpd. The point for me was that the _driver_ can separate hpd handling from polling. Which it still can with this patch applied. The issue at hand is that the old poll=0 also managed to paper over some hpd handling irq storms and so breaking that is a regression. To fix that we should imo apply this patch. We can revert this again once i915 has the hpd irq storm rate-limiting stuff applied (presuming there's no other bug report for other drivers). Also in 3.9 we have the reworked kms locking, so the usual reason that polling causes cursor/pageflip stalls to set poll=0 evaporated. So imo the only people who should still set poll=0 on 3.9 are those with irq storms. And we've just broken that part ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
It will be only consistent once we've restored all the crtcs. Since a
bunch of other callers also want to just restore a single crtc, add a
boolean to disable checking only where it doesn't make sense.
Note that intel_modeset_setup_hw_state already has a call to
intel_modeset_check_state at the end, so we don't reduce the amount of
checking.
v2: Try harder not to create a big patch (Chris).
References: https://lkml.org/lkml/2013/3/16/60
Cc: Tomas Melin
Cc: Richard Cochran
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 34 +++---
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8809813..2959fb8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev)
}
}
-int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb)
+static int __intel_set_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode,
+ int x, int y, struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -8012,8 +8012,6 @@ done:
if (ret && crtc->enabled) {
crtc->hwmode = *saved_hwmode;
crtc->mode = *saved_mode;
- } else {
- intel_modeset_check_state(dev);
}
out:
@@ -8022,9 +8020,25 @@ out:
return ret;
}
+int intel_set_mode(struct drm_crtc *crtc,
+struct drm_display_mode *mode,
+int x, int y, struct drm_framebuffer *fb)
+{
+ int ret;
+
+ ret = __intel_set_mode(crtc, mode, x, y, fb);
+
+ if (ret == 0)
+ intel_modeset_check_state(crtc->dev);
+
+ return ret;
+}
+
void intel_crtc_restore_mode(struct drm_crtc *crtc)
{
- intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
+ __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
+
+ intel_modeset_check_state(crtc->dev);
}
#undef for_each_intel_crtc_masked
@@ -9333,10 +9347,16 @@ setup_pipes:
}
if (force_restore) {
+ /*
+* We need to use raw interfaces for restoring state to avoid
+* checking (bogus) intermediate states.
+*/
for_each_pipe(pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
- intel_crtc_restore_mode(crtc);
+
+ __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+crtc->fb);
}
list_for_each_entry(plane, &dev->mode_config.plane_list, head)
intel_plane_restore(plane);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
On Wed, Apr 10, 2013 at 01:59:02PM +0200, Richard Cochran wrote: > On Tue, Apr 09, 2013 at 09:51:30PM +0200, Daniel Vetter wrote: > > It will be only consistent once we've restored all the crtcs. Since a > > bunch of other callers also want to just restore a single crtc, add a > > boolean to disable checking only where it doesn't make sense. > > > > Note that intel_modeset_setup_hw_state already has a call to > > intel_modeset_check_state at the end, so we don't reduce the amount of > > checking. > > > > v2: Try harder not to create a big patch (Chris). > > To what does tree does this patch apply? > > Tried v3.8.6 and master d02a9a89. It's written against drm-intel-next-queued at http://cgit.freedesktop.org/~danvet/drm-intel I've thought that it should apply pretty cleanly against older kernels, too. Apparently it conflicts a bit in intel_modeset_setup_hw_state, you can just do the s/intel_crtc_restore_mode/__intel_set_mode/ change manually. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
On Wed, Apr 10, 2013 at 7:27 PM, Richard Cochran wrote: > On Wed, Apr 10, 2013 at 02:07:24PM +0200, Daniel Vetter wrote: >> >> It's written against drm-intel-next-queued at >> >> http://cgit.freedesktop.org/~danvet/drm-intel >> >> I've thought that it should apply pretty cleanly against older kernels, >> too. Apparently it conflicts a bit in intel_modeset_setup_hw_state, you >> can just do the s/intel_crtc_restore_mode/__intel_set_mode/ change >> manually. > > I couldn't see right away how to fix it up, so I just compiled your > drm-intel-next-queued plus this patch. If I close the netbook's lid > and open it again, the screen is blank, no backlight, and the machine > seems to be frozen. > > I think I can live with the warning. That patch should crash at all, so this is not expected. Can you pls check whether plain drm-intel-nightly is broken, too? I'll quickly port the patch (in it's latest v3 version) to 3.9-rc kernels for you to test. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
It will be only consistent once we've restored all the crtcs. Since a
bunch of other callers also want to just restore a single crtc, add a
boolean to disable checking only where it doesn't make sense.
Note that intel_modeset_setup_hw_state already has a call to
intel_modeset_check_state at the end, so we don't reduce the amount of
checking.
v2: Try harder not to create a big patch (Chris).
v3: Even smaller (still Chris). Also fix a trailing space.
v4: Rebased on top of 3.9-rc6.
References: https://lkml.org/lkml/2013/3/16/60
Cc: Tomas Melin
Cc: Richard Cochran
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b20d501..5f1eb50 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7771,9 +7771,9 @@ intel_modeset_check_state(struct drm_device *dev)
}
}
-int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb)
+static int __intel_set_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode,
+ int x, int y, struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -7863,8 +7863,6 @@ done:
if (ret && crtc->enabled) {
crtc->hwmode = *saved_hwmode;
crtc->mode = *saved_mode;
- } else {
- intel_modeset_check_state(dev);
}
out:
@@ -7872,6 +7870,20 @@ out:
return ret;
}
+int intel_set_mode(struct drm_crtc *crtc,
+struct drm_display_mode *mode,
+int x, int y, struct drm_framebuffer *fb)
+{
+ int ret;
+
+ ret = __intel_set_mode(crtc, mode, x, y, fb);
+
+ if (ret == 0)
+ intel_modeset_check_state(crtc->dev);
+
+ return ret;
+}
+
void intel_crtc_restore_mode(struct drm_crtc *crtc)
{
intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
@@ -9172,8 +9184,16 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
}
if (force_restore) {
+ /*
+* We need to use raw interfaces for restoring state to avoid
+* checking (bogus) intermediate states.
+*/
for_each_pipe(pipe) {
-
intel_crtc_restore_mode(dev_priv->pipe_to_crtc_mapping[pipe]);
+ struct drm_crtc *crtc =
+ dev_priv->pipe_to_crtc_mapping[pipe];
+
+ __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+crtc->fb);
}
i915_redisable_vga(dev);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
On Thu, Apr 11, 2013 at 7:52 PM, Richard Cochran wrote: > On Wed, Apr 10, 2013 at 10:03:25PM +0200, Daniel Vetter wrote: > >> That patch should crash at all, so this is not expected. Can you pls >> check whether plain drm-intel-nightly is broken, too? > > I did try drm-intel-nightly just now (1dd83e3), and it also freezes > the machine. I first verified that the power button shutdown is > working (before starting X). Then, with X running, closing and > reopening the lid results in a blank screen (no backlight) and a > seemingly frozen box. I've just tracked down and fixed an bug which can lead to a hard-hang in the crtc restore code (which is used both in the lid handler when opening and on resume). If you could please test this patch (on top of drm-intel-nightly): https://patchwork.kernel.org/patch/2428971/ >> I'll quickly port the patch (in it's latest v3 version) to 3.9-rc >> kernels for you to test. > > Okay, I'll try this next. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
It will be only consistent once we've restored all the crtcs. Since a
bunch of other callers also want to just restore a single crtc, add a
boolean to disable checking only where it doesn't make sense.
Note that intel_modeset_setup_hw_state already has a call to
intel_modeset_check_state at the end, so we don't reduce the amount of
checking.
v2: Try harder not to create a big patch (Chris).
v3: Even smaller (still Chris). Also fix a trailing space.
References: https://lkml.org/lkml/2013/3/16/60
Cc: Tomas Melin
Cc: Richard Cochran
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 30 --
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8809813..457a0a0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev)
}
}
-int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb)
+static int __intel_set_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode,
+ int x, int y, struct drm_framebuffer *fb)
{
struct drm_device *dev = crtc->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -8012,8 +8012,6 @@ done:
if (ret && crtc->enabled) {
crtc->hwmode = *saved_hwmode;
crtc->mode = *saved_mode;
- } else {
- intel_modeset_check_state(dev);
}
out:
@@ -8022,6 +8020,20 @@ out:
return ret;
}
+int intel_set_mode(struct drm_crtc *crtc,
+struct drm_display_mode *mode,
+int x, int y, struct drm_framebuffer *fb)
+{
+ int ret;
+
+ ret = __intel_set_mode(crtc, mode, x, y, fb);
+
+ if (ret == 0)
+ intel_modeset_check_state(crtc->dev);
+
+ return ret;
+}
+
void intel_crtc_restore_mode(struct drm_crtc *crtc)
{
intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
@@ -9333,10 +9345,16 @@ setup_pipes:
}
if (force_restore) {
+ /*
+* We need to use raw interfaces for restoring state to avoid
+* checking (bogus) intermediate states.
+*/
for_each_pipe(pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
- intel_crtc_restore_mode(crtc);
+
+ __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+crtc->fb);
}
list_for_each_entry(plane, &dev->mode_config.plane_list, head)
intel_plane_restore(plane);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: don't check inconsistent modeset state when force-restoring
On Thu, Apr 11, 2013 at 08:17:42PM +0100, Chris Wilson wrote: > On Thu, Apr 11, 2013 at 08:22:50PM +0200, Daniel Vetter wrote: > > It will be only consistent once we've restored all the crtcs. Since a > > bunch of other callers also want to just restore a single crtc, add a > > boolean to disable checking only where it doesn't make sense. > > > > Note that intel_modeset_setup_hw_state already has a call to > > intel_modeset_check_state at the end, so we don't reduce the amount of > > checking. > > > > v2: Try harder not to create a big patch (Chris). > > > > v3: Even smaller (still Chris). Also fix a trailing space. > > > > References: https://lkml.org/lkml/2013/3/16/60 > > Cc: Tomas Melin > > Cc: Richard Cochran > > Cc: Chris Wilson > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter > > I'm happy that you are not touching any paths other than mentioned in > the changelog, and that I think being quiet on the modeset is a good use > of the 'force' semantics, so > > Reviewed-by: Chris Wilson Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Fixup Oops in the pipe config computation
Yet again our current confusion between doing the modeset globally, but only having the new parameters for one crtc at a time. So that intel_set_mode essentially already does a global modeset: intel_modeset_affected_pipes compares the current state with where we want to go to (which is carefully set up by intel_crtc_set_config) and then goes through the modeset sequence for any crtc which needs updating. Now the issue is that the actual interface with the remaining code still only works on one crtc, and so we only pass in one fb and one mode. In intel_set_mode we also only compute one intel_crtc_config (which should be the one for the crtc we're doing a modeset on). The reason for that mismatch is twofold: - We want to eventually do all modeset as global state changes, so it's just infrastructure prep. - But even the old semantics can change more than one crtc when you e.g. move a connector from crtc A to crtc B, then both crtc A and B need to be updated. Usually that means one pipe is disabled and the other enabled. This is also the reason why the hack doesn't touch the disable_pipes mask. Now hilarity ensued in our kms config restore paths when we actually try to do a modeset on all crtcs: If the first crtc should be off and the second should be on, then the call on the first crtc will notice that the 2nd one should be switched on and so tries to compute the pipe_config. But due to a lack of passed-in fb (crtc 1 should be off after all) it only results in tears. This case is ridiculously easy to hit on gen2/3 where the lvds output is restricted to pipe B. Note that before the pipe_config bpp rework gen2/3 didn't care really about the fb->depth, so this is a regression brought to light with commit 4e53c2e010e531b4a014692199e978482d471c7e Author: Daniel Vetter Date: Wed Mar 27 00:44:58 2013 +0100 drm/i915: precompute pipe bpp before touching the hw But apparently Ajax also managed to blow up pch platforms, probably with some randomized configs, and pch platforms trip up over the lack of an fb even in the old code. So this actually goes back to the first introduction of the new modeset restore code in commit 45e2b5f640b3766da3eda48f6c35f088155c06f3 Author: Daniel Vetter Date: Fri Nov 23 18:16:34 2012 +0100 drm/i915: force restore on lid open Fix this mess by now by justing shunting all the cool new global modeset logic in intel_modeset_affected_pipes. v2: Improve commit message and clean up all the comments in intel_modeset_affected_pipes - since the introduction of the modeset restore code they've been a bit outdated. Bugzill: https://bugzilla.redhat.com/show_bug.cgi?id=917725 Cc: stable@vger.kernel.org References: http://www.mail-archive.com/stable@vger.kernel.org/msg38084.html Tested-by: Richard Cochran Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1b2c744..010115c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7613,22 +7613,25 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes, if (crtc->enabled) *prepare_pipes |= 1 << intel_crtc->pipe; - /* We only support modeset on one single crtc, hence we need to do that -* only for the passed in crtc iff we change anything else than just -* disable crtcs. -* -* This is actually not true, to be fully compatible with the old crtc -* helper we automatically disable _any_ output (i.e. doesn't need to be -* connected to the crtc we're modesetting on) if it's disconnected. -* Which is a rather nutty api (since changed the output configuration -* without userspace's explicit request can lead to confusion), but -* alas. Hence we currently need to modeset on all pipes we prepare. */ + /* +* For simplicity do a full modeset on any pipe where the output routing +* changed. We could be more clever, but that would require us to be +* more careful with calling the relevant encoder->mode_set functions. +*/ if (*prepare_pipes) *modeset_pipes = *prepare_pipes; /* ... and mask these out. */ *modeset_pipes &= ~(*disable_pipes); *prepare_pipes &= ~(*disable_pipes); + + /* +* HACK: We don't (yet) fully support global modesets. intel_set_config +* obies this rule, but the modeset restore mode of +* intel_modeset_setup_hw_state does not. +*/ + *modeset_pipes &= 1 << intel_crtc->pipe; + *prepare_pipes &= 1 << intel_crtc->pipe; } static bool intel_crtc_in_use(struct drm_crtc *crtc) -- 1.7.10.4 -- To unsubscribe from this list: send the
Re: [Intel-gfx] [PATCH 2/3] drm/i915: don't intel_crt_init on any ULT machines
On Mon, Apr 15, 2013 at 09:45:00PM +0100, Chris Wilson wrote: > On Fri, Apr 12, 2013 at 06:16:53PM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni > > > > We may have DDI_BUF_CTL(PORT_A) configured with 2 lanes and still not > > have CRT, so just check for !IS_ULT. This problem happened on a real > > machine and resulted in a very ugly dmesg. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Paulo Zanoni > > This doesn't apply to dinq. The approach I'd favour is to have a > intel_info.has_crt similar to the other feature flags. I'm ok with piling checks on top here for now, I guess we should reconsider once the next hw platforms shows up around the corner ... All patches merged to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 2/3] drm/i915: don't intel_crt_init on any ULT machines
On Tue, Apr 16, 2013 at 3:41 PM, Paulo Zanoni wrote: >> All patches merged to dinq, thanks. > > I thought patch 2 would go to -fixes. We need it even for older Kernels. > This late in the release cycle -fixes is for severe regressions and black-screen level non-regression issues only. Hence merged through -next with cc: stable. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/mm: fix dump table BUG
In
commit 9e8944ab564f2e3dde90a518cd32048c58918608
Author: Chris Wilson
Date: Thu Nov 15 11:32:17 2012 +
drm: Introduce an iterator over holes in the drm_mm range manager
helpers and iterators for hole handling have been introduced with some
debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
which unconditionally tried to compute the size of the very first
hole.
Reported-by: Christopher Harvey
Cc: Christopher Harvey
Cc: Dave Airlie
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_mm.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index db1e2d6..a0ba3a1 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -759,14 +759,15 @@ int drm_mm_dump_table(struct seq_file *m, struct drm_mm
*mm)
{
struct drm_mm_node *entry;
unsigned long total_used = 0, total_free = 0, total = 0;
- unsigned long hole_start, hole_end, hole_size;
+ unsigned long hole_start, hole_end, hole_size = 0;
- hole_start = drm_mm_hole_node_start(&mm->head_node);
- hole_end = drm_mm_hole_node_end(&mm->head_node);
- hole_size = hole_end - hole_start;
- if (hole_size)
+ if (mm->head_node.hole_follows) {
+ hole_start = drm_mm_hole_node_start(&mm->head_node);
+ hole_end = drm_mm_hole_node_end(&mm->head_node);
+ hole_size = hole_end - hole_start;
seq_printf(m, "0x%08lx-0x%08lx: 0x%08lx: free\n",
hole_start, hole_end, hole_size);
+ }
total_free += hole_size;
drm_mm_for_each_node(entry, mm) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/mm: fix dump table BUG
On Sat, Apr 20, 2013 at 12:11 PM, Chris Wilson wrote:
> On Sat, Apr 20, 2013 at 12:08:11PM +0200, Daniel Vetter wrote:
>> In
>>
>> commit 9e8944ab564f2e3dde90a518cd32048c58918608
>> Author: Chris Wilson
>> Date: Thu Nov 15 11:32:17 2012 +
>>
>> drm: Introduce an iterator over holes in the drm_mm range manager
>>
>> helpers and iterators for hole handling have been introduced with some
>> debug BUG_ONs sprinkled over. Unfortunately this broke the mm dumper
>> which unconditionally tried to compute the size of the very first
>> hole.
>>
>> Reported-by: Christopher Harvey
>> Cc: Christopher Harvey
>> Cc: Dave Airlie
>> Cc: Chris Wilson
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Daniel Vetter
>> ---
>> drivers/gpu/drm/drm_mm.c | 11 ++-
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
>> index db1e2d6..a0ba3a1 100644
>> --- a/drivers/gpu/drm/drm_mm.c
>> +++ b/drivers/gpu/drm/drm_mm.c
>> @@ -759,14 +759,15 @@ int drm_mm_dump_table(struct seq_file *m, struct
>> drm_mm *mm)
>> {
>> struct drm_mm_node *entry;
>> unsigned long total_used = 0, total_free = 0, total = 0;
>> - unsigned long hole_start, hole_end, hole_size;
>> + unsigned long hole_start, hole_end, hole_size = 0;
>>
>> - hole_start = drm_mm_hole_node_start(&mm->head_node);
>> - hole_end = drm_mm_hole_node_end(&mm->head_node);
>
> Use __drm_mm_hole_node_start and __drm_mm_hole_node_end instead.
I've figured I could make the condition more symmetric with the one in
the loop, so that both check ->hole_follows.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm: don't check modeset locks in panic handler
Since we know that locking is broken in that case and it's more
important to not flood the dmesg with random gunk.
Cc: Dave Airlie
Cc: Borislav Petkov
References:
https://groups.google.com/forum/?fromgroups=#!topic/linux.kernel/QFzFxSUeV4I
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_crtc.c |4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 792c3e3..3be0802 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -78,6 +78,10 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device
*dev)
{
struct drm_crtc *crtc;
+ /* Locking is currently fubar in the panic handler. */
+ if (oops_in_progress)
+ return;
+
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
WARN_ON(!mutex_is_locked(&crtc->mutex));
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: don't check modeset locks in panic handler
On Thu, May 02, 2013 at 12:13:08PM +0200, Borislav Petkov wrote:
> On Thu, May 02, 2013 at 09:43:05AM +0200, Daniel Vetter wrote:
> > Since we know that locking is broken in that case and it's more
> > important to not flood the dmesg with random gunk.
> >
> > Cc: Dave Airlie
> > Cc: Borislav Petkov
> > References:
> > https://groups.google.com/forum/?fromgroups=#!topic/linux.kernel/QFzFxSUeV4I
>
> Yeah, we have this nice redirector service on k.org which uses the
> Message-ID only and doesn't rely on some external URLs remaining stable.
> You could use that instead:
>
> Link: http://lkml.kernel.org/r/20130502000206.gh15...@pd.tnic
>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_crtc.c |4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 792c3e3..3be0802 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -78,6 +78,10 @@ void drm_warn_on_modeset_not_all_locked(struct
> > drm_device *dev)
> > {
> > struct drm_crtc *crtc;
> >
> > + /* Locking is currently fubar in the panic handler. */
> > + if (oops_in_progress)
> > + return;
> > +
> > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> > WARN_ON(!mutex_is_locked(&crtc->mutex));
>
> Yep, thanks.
>
> Reported-and-tested-by: Borislav Petkov
Thanks for testing, patch applied to drm-intel-fixes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
drm/i915 stable patches
Dear stable team, Please backport commit 4615d4c9e27eda42c3e965f208a4b4065841498c Author: Chris Wilson Date: Mon Apr 8 14:28:40 2013 +0100 drm/i915: Use MLC (l3$) for context objects to all supported stable kernels. It's not just a performance optimization, but seems to prevent gpu hangs, too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64073 Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drm/i915 stable patches
On Mon, May 6, 2013 at 10:33 AM, Daniel Vetter wrote: > Dear stable team, > > Please backport > > commit 4615d4c9e27eda42c3e965f208a4b4065841498c > Author: Chris Wilson > Date: Mon Apr 8 14:28:40 2013 +0100 > > drm/i915: Use MLC (l3$) for context objects > > to all supported stable kernels. It's not just a performance > optimization, but seems to prevent gpu hangs, too. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64073 And another one for 3.8 and older: commit e12a2d53ae45a69aea499b64f75e7222cca0f12f Author: Chris Wilson Date: 2012-11-15 11:32:18 + drm/i915: Fix detection of base of stolen memory Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=53541 Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Don't clobber crtc->fb when queue_flip fails
On Fri, Feb 22, 2013 at 04:36:38PM +0200, Ville Syrjälä wrote: > I don't think anyone should be poking at crtc->fb w/o holding the crtc > mutex. Except that intel_update_fbc() actually does. That thing would > appear to be just broken since it crawls around in the crtc state w/o > proper protection. The fb could even disappear from under it. fbc has terminally broken locking, and that's been the case since forever afaict. Imo the right solution would be to keep around a bit of fbc state with it's own mutex, and update that (with proper locking) from the crtc enable/disable code at the right spots. But as long as fbc is disabled by default I don't care that much. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH v2] drm/i915: Don't clobber crtc->fb when queue_flip fails
On Fri, Feb 22, 2013 at 03:13:00PM +, Chris Wilson wrote: > On Fri, Feb 22, 2013 at 04:53:38PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Restore crtc->fb to the old framebuffer if queue_flip fails. > > > > While at it, kill the pointless intel_fb temp variable. > > > > v2: Update crtc->fb before queue_flip and restore it back > > after a failure. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Ville Syrjälä > > Lgtm, either v1 or v2 gets my > Reviewed-by: Chris Wilson > (with a slight preference towards v2 for the sake of paranoia) Picked up for -fixes, thanks for the patch. And can I wish for an i-g-t for this? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: enable irqs earlier when resuming
We need it to restore the ilk rc6 context, since the gpu wait no requires interrupts. But in general having interrupts around should help in code sanity, since more and more stuff is interrupt driven. This regression has been introduced in commit 3e9605018ab3e333d51cc90fccfde2031886763b Author: Chris Wilson Date: Tue Nov 27 16:22:54 2012 + drm/i915: Rearrange code to only have a single method for waiting upon the ring Like in the driver load code we need to make sure that hotplug interrupts don't cause havoc with our modeset state, hence block them with the existing infrastructure. Again we ignore races where we might loose hotplug interrupts ... Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54691 Cc: stable@vger.kernel.org (for 3.8 only) Cc: Chris Wilson Cc: Mika Kuoppala Reported-and-Tested-by: Ilya Tumaykin Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b342749..7589a2a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -493,6 +493,7 @@ static int i915_drm_freeze(struct drm_device *dev) intel_modeset_disable(dev); drm_irq_uninstall(dev); + dev_priv->enable_hotplug_processing = false; } i915_save_state(dev); @@ -566,10 +567,20 @@ static int __i915_drm_thaw(struct drm_device *dev) error = i915_gem_init_hw(dev); mutex_unlock(&dev->struct_mutex); + /* We need working interrupts for modeset enabling ... */ + drm_irq_install(dev); + intel_modeset_init_hw(dev); intel_modeset_setup_hw_state(dev, false); - drm_irq_install(dev); + + /* +* ... but also need to make sure that hotplug processing +* doesn't cause havoc. Like in the driver load code we don't +* bother with the tiny race here where we might loose hotplug +* notifications. +* */ intel_hpd_init(dev); + dev_priv->enable_hotplug_processing = true; } intel_opregion_init(dev); -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: enable irqs earlier when resuming
On Tue, Mar 05, 2013 at 08:53:48AM +, Chris Wilson wrote: > On Tue, Mar 05, 2013 at 09:50:58AM +0100, Daniel Vetter wrote: > > We need it to restore the ilk rc6 context, since the gpu wait no > > requires interrupts. But in general having interrupts around should > > help in code sanity, since more and more stuff is interrupt driven. > > > > This regression has been introduced in > > > > commit 3e9605018ab3e333d51cc90fccfde2031886763b > > Author: Chris Wilson > > Date: Tue Nov 27 16:22:54 2012 + > > > > drm/i915: Rearrange code to only have a single method for waiting upon > > the ring > > Not entirely, as we can also regard this as an oversight from fixing up > the irq sequence during initialisation... :-p Added a citation to the relevant commit ... > > > > Like in the driver load code we need to make sure that hotplug > > interrupts don't cause havoc with our modeset state, hence block them > > with the existing infrastructure. Again we ignore races where we might > > loose hotplug interrupts ... > > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=54691 > > Cc: stable@vger.kernel.org (for 3.8 only) > > Cc: Chris Wilson > > Cc: Mika Kuoppala > > Reported-and-Tested-by: Ilya Tumaykin > > Signed-off-by: Daniel Vetter > > Reviewed-by: Chris wilson ... and merged to -fixes, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Revert a bunch of patches in stable kernels
Hi Greg&all, So a recent stable backport to fix rc6 on ilk (which is disabled by default and with dubious power savings at best, unlike rc6 on snb and later) totally blew up all over the place: https://bugzilla.kernel.org/show_bug.cgi?id=55291 https://lkml.org/lkml/2013/3/14/540 There might be more, I'm still recovering from mail floods due to traveling last week. I think the right course of action is to revert the offending patch (plus anything depending upon it) and give up on rc6 on ilk in 3.8 - too messy. All bug reports confirmed that 3.9-rc kernels work as expected. Upstream commits to revert: 15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9 drm/i915: enable irqs earlier when resuming 52d7ecedac3f96fb562cb482c139015372728638 drm/i915: reorder setup sequence to have irqs for output setup Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Revert a bunch of patches in stable kernels
On Mon, Mar 18, 2013 at 11:05 AM, Daniel Vetter wrote: > Hi Greg&all, > > So a recent stable backport to fix rc6 on ilk (which is disabled by > default and with dubious power savings at best, unlike rc6 on snb and > later) totally blew up all over the place: > https://bugzilla.kernel.org/show_bug.cgi?id=55291 > https://lkml.org/lkml/2013/3/14/540 This likely also is the culprit behind the ghost vga issues reported on gm45, e.g: https://bugzilla.redhat.com/show_bug.cgi?id=922304 Again, fixed in 3.9-rc kernels by commit 20afbda209d708be66944907966486d0c1331cb8 Author: Daniel Vetter Date: Tue Dec 11 14:05:07 2012 +0100 drm/i915: Fixup hpd irq register setup ordering Cheers, Daniel > There might be more, I'm still recovering from mail floods due to > traveling last week. I think the right course of action is to revert > the offending patch (plus anything depending upon it) and give up on > rc6 on ilk in 3.8 - too messy. All bug reports confirmed that 3.9-rc > kernels work as expected. Upstream commits to revert: > > 15239099d7a7a9ecdc1ccb5b187ae4cda5488ff9 drm/i915: enable irqs earlier > when resuming > 52d7ecedac3f96fb562cb482c139015372728638 drm/i915: reorder setup > sequence to have irqs for output setup > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Stable kernel 3.8.3 appears to break displayport on intel gen4
On Mon, Mar 18, 2013 at 04:59:35PM +0100, Bjørn Mork wrote:
> Sergio Callegari writes:
>
> > This is just a short note to let you know that after installing 3.8.3,
> > display port stopped working on my laptop. Going back to 3.8.2 brought
> > it back to life.
> > This has been tested with the ubuntu mainline kernels that should be
> > vanilla stable kernels. Hope this is not an incorrect report due to
> > something wrong on their side. Laptop is a DELL E6500 with intel gen4
> > integrated graphics (Intel Corporation Mobile 4 Series Chipset
> > Integrated Graphics Controller (rev 07)).
> > With 3.8.3, xrandr does not report anymore the external monitor when
> > it is actually attached via displayport.
>
> I can confirm seeing this bug on:
>
> # lspci -nnvvvs 00:02.0
> 00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 4 Series
> Chipset Integrated Graphics Controller [8086:2a42] (rev 07) (prog-if 00 [VGA
> controller])
> Subsystem: Lenovo Device [17aa:20e4]
> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx+
> Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort-
> SERR- Latency: 0
> Interrupt: pin A routed to IRQ 45
> Region 0: Memory at f000 (64-bit, non-prefetchable) [size=4M]
> Region 2: Memory at d000 (64-bit, prefetchable) [size=256M]
> Region 4: I/O ports at 1800 [size=8]
> Expansion ROM at [disabled]
> Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
> Address: fee0300c Data: 4152
> Capabilities: [d0] Power Management version 3
> Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot-,D3cold-)
> Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> Kernel driver in use: i915
>
>
>
> Looking through the changes from v3.8.2 to v3.8.3, there weren't really
> that many suspects. This partial (to avoid having to change any
> following patches) revert of commit 2a98104 ("drm/i915: reorder setup
> sequence to have irqs for output setup") fixes the problem for me. I
> have no idea why, so I leave it to Daniel and the other wise men working
> on this driver to explain and cleanup :)
Already taken care of hopefully:
http://lists.freedesktop.org/archives/intel-gfx/2013-March/025767.html
My apologies to everyone who's suffering through this breakage, it looks
like I've managed to serious burn myself with an innocent looking stable
backport :(
Yours, Daniel
>
>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5206f24..b15b65d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1297,21 +1297,19 @@ static int i915_load_modeset_init(struct drm_device
> *dev)
> if (ret)
> goto cleanup_vga_switcheroo;
>
> - ret = drm_irq_install(dev);
> - if (ret)
> - goto cleanup_gem_stolen;
> -
> - /* Important: The output setup functions called by modeset_init need
> - * working irqs for e.g. gmbus and dp aux transfers. */
> intel_modeset_init(dev);
>
> ret = i915_gem_init(dev);
> if (ret)
> - goto cleanup_irq;
> + goto cleanup_gem_stolen;
> +
> + intel_modeset_gem_init(dev);
>
> INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
>
> - intel_modeset_gem_init(dev);
> + ret = drm_irq_install(dev);
> + if (ret)
> + goto cleanup_gem;
>
> /* Always safe in the mode setting case. */
> /* FIXME: do pre/post-mode set stuff in core KMS code */
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations
On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Cc: stable@vger.kernel.org
One of the stable rules is that patches should fix real issues. So can you
please hunt through bugzillas quickly and feed this to relevant bug
reports?
-Daniel
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8a3d89e..89a2d6f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1943,7 +1943,7 @@ static void ivybridge_update_wm(struct drm_device *dev)
> DRM_DEBUG_KMS("FIFO watermarks For pipe C -"
> " plane %d, cursor: %d\n",
> plane_wm, cursor_wm);
> - enabled |= 3;
> + enabled |= 4;
> }
>
> /*
> --
> 1.8.1.5
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Fix pipe enabled mask for pipe C in WM calculations
On Wed, Mar 20, 2013 at 11:39 PM, Chris Wilson wrote: > On Wed, Mar 20, 2013 at 11:05:37PM +0100, Daniel Vetter wrote: >> On Wed, Mar 20, 2013 at 09:51:05PM +0200, ville.syrj...@linux.intel.com >> wrote: >> > From: Ville Syrjälä >> > >> > Cc: stable@vger.kernel.org >> >> One of the stable rules is that patches should fix real issues. So can you >> please hunt through bugzillas quickly and feed this to relevant bug >> reports? > > It would take an astute user to notice that his machine was not using a > lower power self-refresh FIFO mode. And the number of machines that only > set up the third pipe is going to be small, so the impact minor. It > will not fix any of the three pipe issues we have open, for example. Ah, lazy me didn't read the docs. Low power fifo mode is indeed hard to report in a bug ;-) > The patch is correct, though had I used enabled |= 1 << PIPE_[ABC] it > would have probably prevented this bug. This would be quite a bit less magic I think. Can I have it, please? -Daniel > Reviewed-by: Chris Wilson -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Workaround incoherence between fences and LLC across multiple CPUs
On Thu, Mar 21, 2013 at 03:30:19PM +, Chris Wilson wrote:
> In order to fully serialize access to the fenced region and the update
> to the fence register we need to take extreme measures on SNB+, and
> write the fence from each cpu taking care to serialise memory accesses
> on each. The usual mb(), or even a mb() on each CPU is not enough to
> ensure that access to the fenced region is coherent across the change in
> fence register.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
> Signed-off-by: Chris Wilson
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/i915_gem.c | 34 ++
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 19fc21b..fe34240 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2684,17 +2684,43 @@ static inline int fence_number(struct
> drm_i915_private *dev_priv,
> return fence - dev_priv->fence_regs;
> }
>
> +struct write_fence {
> + struct drm_device *dev;
> + struct drm_i915_gem_object *obj;
> + int fence;
> +};
> +
> +static void i915_gem_write_fence__ipi(void *data)
> +{
> + struct write_fence *args = data;
> + i915_gem_write_fence(args->dev, args->fence, args->obj);
> +}
> +
> static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
>struct drm_i915_fence_reg *fence,
>bool enable)
> {
> struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> - int reg = fence_number(dev_priv, fence);
> -
> - i915_gem_write_fence(obj->base.dev, reg, enable ? obj : NULL);
> + struct write_fence args = {
> + .dev = obj->base.dev,
> + .fence = fence_number(dev_priv, fence),
> + .obj = enable ? obj : NULL,
> + };
> +
> + /* In order to fully serialize access to the fenced region and
> + * the update to the fence register we need to take extreme
> + * measures on SNB+, and write the fence from each cpu taking
> + * care to serialise memory accesses on each. The usual mb(),
> + * or even a mb() on each CPU is not enough to ensure that access
> + * to the fenced region is coherent across the change in fence
> + * register.
> + */
> + if (!HAS_LLC(obj->base.dev) ||
> + on_each_cpu(i915_gem_write_fence__ipi, &args, 1) != 0)
> + i915_gem_write_fence__ipi(&args);
I think the if condition here is a notch to clever and hides the
elefantentöter a bit too well. on_each_cpu always calls the given function
unconditionally, even when the ipi function call fails, so
if (!HAS_LLC)
WARN_ON(on_each_cpu);
else
i915_gem_write_fence
looks clearer to me.
-Daniel
>
> if (enable) {
> - obj->fence_reg = reg;
> + obj->fence_reg = args.fence;
> fence->obj = obj;
> list_move_tail(&fence->lru_list, &dev_priv->mm.fence_list);
> } else {
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Always call fence-lost prior to removing the fence
On Tue, Mar 26, 2013 at 04:25:58PM +0200, Imre Deak wrote: > On Tue, 2013-03-26 at 11:29 +, Chris Wilson wrote: > > There is a minute window for a race between put-fence removing the fence > > and for a new transaction by an external party on the GTT mmap. That is > > we must zap the mmap prior to removing the fence and not afterwards. > > > > Fixes regression from > > commit 61050808bb019ebea966b7b5bfd357aaf219fb51 > > Author: Chris Wilson > > Date: Tue Apr 17 15:31:31 2012 +0100 > > > > drm/i915: Refactor put_fence() to use the common fence writing routine > > > > v2: Remember the fence to remove with a local variable (gcc) > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: stable@vger.kernel.org # regression introduced in v3.5 > > Reviewed-by: Imre Deak stable rules say "no theoretical races", and I think we don't even have a testcase for this. Hence queued for -next and dropped cc: stable, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ 74/75] Revert "drm/i915: enable irqs earlier when resuming"
On Wed, Mar 27, 2013 at 4:45 PM, Ilya Tumaykin wrote: > On 18.03.13 14:07:37 Greg Kroah-Hartman wrote: >> 3.8-stable review patch. If anyone has any objections, please let me know. > > Hello, Greg. > > The original "drm/i915: enable irqs earlier when resuming" commit was > introduced in order to fix bug appeared in 3.8 kernels. Reverting this commit > you also bring back that bug. Bugzilla link: > https://bugzilla.kernel.org/show_bug.cgi?id=54691#c17 > > Sorry for late reply, I wasn't sure if this is simple revert or another > mechanism was introduced by devs to avoid mentioned bug and that fix became > unnecessary. Please undo this revert. Nack. This thing blew up all over the place and caused about 5 different regressions. It works all correctly in 3.9 due to the different context (completely revamped init sequence around irq handling). I realize that this means that ilk rc6 is broken in 3.8 (and we can't really fix it without either going back to the ilk rc6 code in 3.6 or copying the entire 3.9 irq init sequence, both a bit too intrusive for stable). But: - ilk rc6 is known to hang machines - ilk rc6 is (opposed to rc6 on snb+) not know to do anything good like safe power And so disabled by default. So I hope that regression is bearable, feel free to beat the drm/i915 guys as usual for their failings ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"
On Thu, Jan 10, 2013 at 1:23 AM, Chris Wilson wrote: > On Wed, 9 Jan 2013 18:57:09 +0100, Daniel Vetter > wrote: >> This partially reverts >> >> commit 6c085a728cf000ac1865d66f8c9b52935558b328 >> Author: Chris Wilson >> Date: Mon Aug 20 11:40:46 2012 +0200 >> >> drm/i915: Track unbound pages >> >> Closer inspection of that patch revealed a bunch of unrelated changes >> in the shrinker: >> - The shrinker count is now in pages instead of objects. >> - For counting the shrinkable objects the old code only looked at the >> inactive list, the new code looks at all bounds objects (including >> pinned ones). That is obviously in addition to the new unbound list. >> - The shrinker cound is no longer scaled with >> sysctl_vfs_cache_pressure. > > I made a mistake and copied the wrong code in my original > implementation: > > vfs_cache_pressure > -- > > Controls the tendency of the kernel to reclaim the memory which is used for > caching of directory and inode objects. > > At the default value of vfs_cache_pressure=100 the kernel will attempt to > reclaim dentries and inodes at a "fair" rate with respect to pagecache and > swapcache reclaim. Decreasing vfs_cache_pressure causes the kernel to prefer > to retain dentry and inode caches. When vfs_cache_pressure=0, the kernel will > never reclaim dentries and inodes due to memory pressure and this can easily > lead to out-of-memory conditions. Increasing vfs_cache_pressure beyond 100 > causes the kernel to prefer to reclaim dentries and inodes. > > > > It's not an inode or a directory and our pages directly akin to the page > and buffer caches, so I should have never used vfs_cache_pressure and > you have not justified why you are adding it back. Yeah, I agree that the vfs_cache_pressure tuning is rather bogus, but I'd prefer to revert all the shrinker related changes for now. We can remedy this in next with a quick patch easily - burned child applies ;-) >> - When actually shrinking objects, the old code first dropped >> purgeable objects, then normal (inactive) objects. Only then did it, >> in a last-ditch effort idle the gpu and evict everything. The new >> code omits the intermediate step of evicting normal inactive >> objects. > > This is the crux of the papering, so just do this and call it what it is. Agreed, I'll clarify the commit message a bit and stress that the referenced bugs aren't really fixed, but at least the regressions should be resolved. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"
This partially reverts
commit 6c085a728cf000ac1865d66f8c9b52935558b328
Author: Chris Wilson
Date: Mon Aug 20 11:40:46 2012 +0200
drm/i915: Track unbound pages
Closer inspection of that patch revealed a bunch of unrelated changes
in the shrinker:
- The shrinker count is now in pages instead of objects.
- For counting the shrinkable objects the old code only looked at the
inactive list, the new code looks at all bounds objects (including
pinned ones). That is obviously in addition to the new unbound list.
- The shrinker cound is no longer scaled with
sysctl_vfs_cache_pressure. Note though that with the default tuning
value of vfs_cache_pressue = 100 this doesn't affect the shrinker
behaviour.
- When actually shrinking objects, the old code first dropped
purgeable objects, then normal (inactive) objects. Only then did it,
in a last-ditch effort idle the gpu and evict everything. The new
code omits the intermediate step of evicting normal inactive
objects.
Safe for the first change, which seems benign, and the shrinker count
scaling, which is a bit a different story, the endresult of all these
changes is that the shrinker is _much_ more likely to fall back to the
last-ditch resort of idling the gpu and evicting everything. The old
code could only do that if something else evicted lots of objects
meanwhile (since without any other changes the nr_to_scan will be
smaller than the object count).
Reverting the vfs_cache_pressure behaviour itself is a bit bogus, but
I've figured I'll just revert all the shrinker changes which might
affect behaviour: Only dentry/inode object caches should scale their
shrinker counts with vfs_cache_pressure. But such a change is better
done through -next, with a proper changelog.
Hence revert all these changes and restore the old shrinker behaviour,
with the minor adjustment that we now first scan the unbound list,
then the inactive list for each object category (purgeable or normal).
A similar patch has been tested by a few people affected by the gen4/5
hangs which started to appear in 3.7, which some people bisected to
the "drm/i915: Track unbound pages" commit. But just disabling the
unbound logic alone didn't change things at all.
Note that this patch doesn't fix the referenced bugs, it only hides
the underlying bug(s) well enough to restore pre-3.7 behaviour. The
key to achieve that is to massively reduce the likelyhood of going
into a full gpu stall and evicting everything.
Tested-by: Greg KH
Tested-by: Dave Kleikamp
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
References: https://bugs.freedesktop.org/show_bug.cgi?id=57122
References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=57136
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 51df4ee..7c42788 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1714,7 +1714,8 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
}
static long
-i915_gem_purge(struct drm_i915_private *dev_priv, long target)
+__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
+ bool purgeable_only)
{
struct drm_i915_gem_object *obj, *next;
long count = 0;
@@ -1722,7 +1723,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.unbound_list,
gtt_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
if (count >= target)
@@ -1733,7 +1734,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.inactive_list,
mm_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_unbind(obj) == 0 &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
@@ -1745,6 +1746,12 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
return count;
}
+static long
+i915_gem_purge(struct drm_i915_private *dev_priv, long target)
+{
+ return __i915_gem_shrink(dev_priv, target, true);
+}
+
static void
i915_gem_shrink_all(struct dr
[PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"
This partially reverts
commit 6c085a728cf000ac1865d66f8c9b52935558b328
Author: Chris Wilson
Date: Mon Aug 20 11:40:46 2012 +0200
drm/i915: Track unbound pages
Closer inspection of that patch revealed a bunch of unrelated changes
in the shrinker:
- The shrinker count is now in pages instead of objects.
- For counting the shrinkable objects the old code only looked at the
inactive list, the new code looks at all bounds objects (including
pinned ones). That is obviously in addition to the new unbound list.
- The shrinker cound is no longer scaled with
sysctl_vfs_cache_pressure. Note though that with the default tuning
value of vfs_cache_pressue = 100 this doesn't affect the shrinker
behaviour.
- When actually shrinking objects, the old code first dropped
purgeable objects, then normal (inactive) objects. Only then did it,
in a last-ditch effort idle the gpu and evict everything. The new
code omits the intermediate step of evicting normal inactive
objects.
Safe for the first change, which seems benign, and the shrinker count
scaling, which is a bit a different story, the endresult of all these
changes is that the shrinker is _much_ more likely to fall back to the
last-ditch resort of idling the gpu and evicting everything. The old
code could only do that if something else evicted lots of objects
meanwhile (since without any other changes the nr_to_scan will be
smaller than the object count).
Reverting the vfs_cache_pressure behaviour itself is a bit bogus, but
I've figured I'll just revert all the shrinker changes which might
affect behaviour: Only dentry/inode object caches should scale their
shrinker counts with vfs_cache_pressure. But such a change is better
done through -next, with a proper changelog.
Hence revert all these changes and restore the old shrinker behaviour,
with the minor adjustment that we now first scan the unbound list,
then the inactive list for each object category (purgeable or normal).
A similar patch has been tested by a few people affected by the gen4/5
hangs which started to appear in 3.7, which some people bisected to
the "drm/i915: Track unbound pages" commit. But just disabling the
unbound logic alone didn't change things at all.
Note that this patch doesn't fix the referenced bugs, it only hides
the underlying bug(s) well enough to restore pre-3.7 behaviour. The
key to achieve that is to massively reduce the likelyhood of going
into a full gpu stall and evicting everything.
v2: Reword commit message a bit, taking Chris Wilson's comment into
account.
Tested-by: Greg KH
Tested-by: Dave Kleikamp
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
References: https://bugs.freedesktop.org/show_bug.cgi?id=57122
References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=57136
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 51df4ee..7c42788 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1714,7 +1714,8 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
}
static long
-i915_gem_purge(struct drm_i915_private *dev_priv, long target)
+__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
+ bool purgeable_only)
{
struct drm_i915_gem_object *obj, *next;
long count = 0;
@@ -1722,7 +1723,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.unbound_list,
gtt_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
if (count >= target)
@@ -1733,7 +1734,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.inactive_list,
mm_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_unbind(obj) == 0 &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
@@ -1745,6 +1746,12 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
return count;
}
+static long
+i915_gem_purge(struct drm_i915_private *dev_priv, long target)
+{
+ return
[PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"
This partially reverts
commit 6c085a728cf000ac1865d66f8c9b52935558b328
Author: Chris Wilson
Date: Mon Aug 20 11:40:46 2012 +0200
drm/i915: Track unbound pages
Closer inspection of that patch revealed a bunch of unrelated changes
in the shrinker:
- The shrinker count is now in pages instead of objects.
- For counting the shrinkable objects the old code only looked at the
inactive list, the new code looks at all bounds objects (including
pinned ones). That is obviously in addition to the new unbound list.
- The shrinker cound is no longer scaled with
sysctl_vfs_cache_pressure. Note though that with the default tuning
value of vfs_cache_pressue = 100 this doesn't affect the shrinker
behaviour.
- When actually shrinking objects, the old code first dropped
purgeable objects, then normal (inactive) objects. Only then did it,
in a last-ditch effort idle the gpu and evict everything. The new
code omits the intermediate step of evicting normal inactive
objects.
Safe for the first change, which seems benign, and the shrinker count
scaling, which is a bit a different story, the endresult of all these
changes is that the shrinker is _much_ more likely to fall back to the
last-ditch resort of idling the gpu and evicting everything. The old
code could only do that if something else evicted lots of objects
meanwhile (since without any other changes the nr_to_scan will be
smaller than the object count).
Reverting the vfs_cache_pressure behaviour itself is a bit bogus: Only
dentry/inode object caches should scale their shrinker counts with
vfs_cache_pressure. Originally I've had that change reverted, too. But
Chris Wilson insisted that it's too bogus and shouldn't again see the
light of day.
Hence revert all these other changes and restore the old shrinker
behaviour, with the minor adjustment that we now first scan the
unbound list, then the inactive list for each object category
(purgeable or normal).
A similar patch has been tested by a few people affected by the gen4/5
hangs which started to appear in 3.7, which some people bisected to
the "drm/i915: Track unbound pages" commit. But just disabling the
unbound logic alone didn't change things at all.
Note that this patch doesn't fix the referenced bugs, it only hides
the underlying bug(s) well enough to restore pre-3.7 behaviour. The
key to achieve that is to massively reduce the likelyhood of going
into a full gpu stall and evicting everything.
v2: Reword commit message a bit, taking Chris Wilson's comment into
account.
v3: On Chris Wilson's insistency, do not reinstate the rather bogus
vfs_cache_pressure change.
Tested-by: Greg KH
Tested-by: Dave Kleikamp
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
References: https://bugs.freedesktop.org/show_bug.cgi?id=57122
References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=57136
Cc: Chris Wilson
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 51df4ee..7a0ea83 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1714,7 +1714,8 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
}
static long
-i915_gem_purge(struct drm_i915_private *dev_priv, long target)
+__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
+ bool purgeable_only)
{
struct drm_i915_gem_object *obj, *next;
long count = 0;
@@ -1722,7 +1723,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.unbound_list,
gtt_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
if (count >= target)
@@ -1733,7 +1734,7 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
list_for_each_entry_safe(obj, next,
&dev_priv->mm.inactive_list,
mm_list) {
- if (i915_gem_object_is_purgeable(obj) &&
+ if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
i915_gem_object_unbind(obj) == 0 &&
i915_gem_object_put_pages(obj) == 0) {
count += obj->base.size >> PAGE_SHIFT;
@@ -1745,6 +1746,12 @@ i915_gem_purge(struct drm_i915_private *dev_priv, long
target)
return count;
}
+static long
+i915_gem_pu
Re: [Intel-gfx] [PATCH] drm/i915: Revert shrinker changes from "Track unbound pages"
On Thu, Jan 10, 2013 at 04:58:30PM +, Chris Wilson wrote: > On Thu, 10 Jan 2013 18:03:00 +0100, Daniel Vetter > wrote: > > This partially reverts > > > > commit 6c085a728cf000ac1865d66f8c9b52935558b328 > > Author: Chris Wilson > > Date: Mon Aug 20 11:40:46 2012 +0200 > > > > drm/i915: Track unbound pages > > > > Closer inspection of that patch revealed a bunch of unrelated changes > > in the shrinker: > > - The shrinker count is now in pages instead of objects. > > - For counting the shrinkable objects the old code only looked at the > > inactive list, the new code looks at all bounds objects (including > > pinned ones). That is obviously in addition to the new unbound list. > > - The shrinker cound is no longer scaled with > > sysctl_vfs_cache_pressure. Note though that with the default tuning > > value of vfs_cache_pressue = 100 this doesn't affect the shrinker > > behaviour. > > - When actually shrinking objects, the old code first dropped > > purgeable objects, then normal (inactive) objects. Only then did it, > > in a last-ditch effort idle the gpu and evict everything. The new > > code omits the intermediate step of evicting normal inactive > > objects. > > > > Safe for the first change, which seems benign, and the shrinker count > > scaling, which is a bit a different story, the endresult of all these > > changes is that the shrinker is _much_ more likely to fall back to the > > last-ditch resort of idling the gpu and evicting everything. The old > > code could only do that if something else evicted lots of objects > > meanwhile (since without any other changes the nr_to_scan will be > > smaller than the object count). > > > > Reverting the vfs_cache_pressure behaviour itself is a bit bogus: Only > > dentry/inode object caches should scale their shrinker counts with > > vfs_cache_pressure. Originally I've had that change reverted, too. But > > Chris Wilson insisted that it's too bogus and shouldn't again see the > > light of day. > > > > Hence revert all these other changes and restore the old shrinker > > behaviour, with the minor adjustment that we now first scan the > > unbound list, then the inactive list for each object category > > (purgeable or normal). > > > > A similar patch has been tested by a few people affected by the gen4/5 > > hangs which started to appear in 3.7, which some people bisected to > > the "drm/i915: Track unbound pages" commit. But just disabling the > > unbound logic alone didn't change things at all. > > > > Note that this patch doesn't fix the referenced bugs, it only hides > > the underlying bug(s) well enough to restore pre-3.7 behaviour. The > > key to achieve that is to massively reduce the likelyhood of going > > into a full gpu stall and evicting everything. > > > > v2: Reword commit message a bit, taking Chris Wilson's comment into > > account. > > > > v3: On Chris Wilson's insistency, do not reinstate the rather bogus > > vfs_cache_pressure change. > > > > Tested-by: Greg KH > > Tested-by: Dave Kleikamp > > References: https://bugs.freedesktop.org/show_bug.cgi?id=55984 > > References: https://bugs.freedesktop.org/show_bug.cgi?id=57122 > > References: https://bugs.freedesktop.org/show_bug.cgi?id=56916 > > References: https://bugs.freedesktop.org/show_bug.cgi?id=57136 > > Cc: Chris Wilson > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter > > Acked-by: Chris Wilson Picked up for -fixes. > I just hope the clue bat descends soonest before we find another way of > triggering the spurious hangs. Indeed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Revert "drm: Add EDID_QUIRK_FORCE_REDUCED_BLANKING for ASUS VW222S"
This reverts commit 6f33814bd4d9cfe76033a31b1c0c76c960cd8e4b.
The quirk cause a regression, and it looks like the original bug was
simply a lack of FIFO bandwidth on the i915G of the reporter. Which
should eventually be fixed as soon as we get around to implemented
DSPARB FIFO reassignment on gen 3.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52281
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_edid.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a3770f..39919f2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -87,9 +87,6 @@ static struct edid_quirk {
int product_id;
u32 quirks;
} edid_quirk_list[] = {
- /* ASUS VW222S */
- { "ACI", 0x22a2, EDID_QUIRK_FORCE_REDUCED_BLANKING },
-
/* Acer AL1706 */
{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
/* Acer F51 */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.7.x] drm/i915: force restore on lid open
Acked.
-Daniel
On Tue, Jan 15, 2013 at 7:29 AM, CAI Qian wrote:
> Hello Daniel, this is pre-requisite for
> 0fde901f1ddd2ce0e380a6444f1fb7ca555859e9
> to be applied to the 3.7.x stable. Also, this alone looks like a valid
> fix for stable while it was missing in the first place. The back-port patch
> used
> i915_drm_thaw() instead __i915_drm_thaw() in the first hunk for the 3.7.x
> code-base. Please help ACK/NAK this.
>
> Regards,
> CAI Qian
>
> From 45e2b5f640b3766da3eda48f6c35f088155c06f3 Mon Sep 17 00:00:00 2001
> From: Daniel Vetter
> Date: Fri, 23 Nov 2012 18:16:34 +0100
> Subject: [PATCH] drm/i915: force restore on lid open
>
> There seem to be indeed some awkwards machines around, mostly those
> without OpRegion support, where the firmware changes the display hw
> state behind our backs when closing the lid.
>
> This force-restore logic has been originally introduced in
>
> commit c1c7af60892070e4b82ad63bbfb95ae745056de0
> Author: Jesse Barnes
> Date: Thu Sep 10 15:28:03 2009 -0700
>
> drm/i915: force mode set at lid open time
>
> but after the modeset-rework we've disabled it in the vain hope that
> it's no longer required:
>
> commit 3b7a89fce3e3dc96b549d6d829387b4439044d0d
> Author: Daniel Vetter
> Date: Mon Sep 17 22:27:21 2012 +0200
>
> drm/i915: fix OOPS in lid_notify
>
> Alas, no.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54677
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57434
> Tested-by: Krzysztof Mazur
> Reviewed-by: Jesse Barnes
> Signed-off-by: Daniel Vetter
> Signed-off-by: CAI Qian
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6770ee6..1f20ead 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -552,7 +552,7 @@ static int i915_drm_thaw(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
>
> intel_modeset_init_hw(dev);
> -intel_modeset_setup_hw_state(dev);
> +intel_modeset_setup_hw_state(dev, false);
> drm_mode_config_reset(dev);
> drm_irq_install(dev);
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f511fa2..92f1750 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1595,7 +1595,8 @@ extern void intel_modeset_init(struct drm_device *dev);
> extern void intel_modeset_gem_init(struct drm_device *dev);
> extern void intel_modeset_cleanup(struct drm_device *dev);
> extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
> -extern void intel_modeset_setup_hw_state(struct drm_device *dev);
> +extern void intel_modeset_setup_hw_state(struct drm_device *dev,
> + bool force_restore);
> extern bool intel_fbc_enabled(struct drm_device *dev);
> extern void intel_disable_fbc(struct drm_device *dev);
> extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 210b758..c6f5ed3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8281,7 +8281,8 @@ static void intel_sanitize_encoder(struct intel_encoder
> *encoder)
>
> /* Scan out the current hw modeset state, sanitizes it and maps it into the
> drm
> * and i915 state tracking structures. */
> -void intel_modeset_setup_hw_state(struct drm_device *dev)
> +void intel_modeset_setup_hw_state(struct drm_device *dev,
> + bool force_restore)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe;
> @@ -8352,7 +8353,15 @@ void intel_modeset_setup_hw_state(struct drm_device
> *dev)
> intel_sanitize_crtc(crtc);
> }
>
> -intel_modeset_update_staged_output_state(dev);
> +if (force_restore) {
> +for_each_pipe(pipe) {
> +crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> +intel_set_mode(&crtc->base, &crtc->base.mode,
> + crtc->base.x, crtc->base.y,
> crtc->base.fb);
> +}
> +} else {
> +intel_modeset_update_staged_output_state(dev);
> +}
>
> intel_modeset_check_state(dev);
> }
> @@ -8363,7 +8372,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
>
> intel_setup_overlay(dev);
>
> -intel_modeset_setup_hw_state(dev)
Re: [PATCH 3.7.x] drm/i915: Treat crtc->mode.clock == 0 as disabled
I seem to suck a bit at properly tagging stuff cc: stable ... thanks
for going through these. Acked.
Btw, I think it's quicker if you just supply a shortlog of all the
patches I've missed, then I can quickly go through them and mass-ack
them ;-)
-Daniel
On Tue, Jan 15, 2013 at 8:49 AM, CAI Qian wrote:
> Daniel, this patch fixed the compilation error by transparently
> applied f20e0b08b8b2a8432e6abf3683960099f0ab2958 (drm/i915: Prefer
> CRTC 'active' rather than 'enabled' during WM computations) first.
> Then, this upstream commit is totally cancelled out the above one
> and applied successfully. Please ACK/NAK it if possible.
>
> Regards,
> CAI Qian
>
> From 3490ea5de6ac4af309c3df8a26a5cca61306334c Mon Sep 17 00:00:00 2001
> From: Chris Wilson
> Date: Mon, 7 Jan 2013 10:11:40 +
> Subject: [PATCH] drm/i915: Treat crtc->mode.clock == 0 as disabled
>
> Prevent a divide-by-zero by consistently treating an 'active' CRTC
> without a mode set as actually disabled.
>
> This looks to have been first introduced with
>
> commit 24929352481f085c5f85d4d4cbc919ddf106d381
> Author: Daniel Vetter
> Date: Mon Jul 2 20:28:59 2012 +0200
>
> drm/i915: read out the modeset hw state at load and resume time
>
> but then combined with
>
> commit b0a2658acb5bf9ca86b4aab011b7106de3af0add
> Author: Daniel Vetter
> Date: Tue Dec 18 09:37:54 2012 +0100
>
> drm/i915: don't disable disconnected outputs
>
> it finally started oopsing.
>
> Signed-off-by: Chris Wilson
> Reported-and-tested-by: Alexey Zaytsev
> Tested-by: Sedat Dilek
> Cc: Daniel Vetter
> Cc: Jesse Barnes
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 442968f..eaaff3c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -44,6 +44,14 @@
> * i915.i915_enable_fbc parameter
> */
>
> +static bool intel_crtc_active(struct drm_crtc *crtc)
> +{
> + /* Be paranoid as we can arrive here with only partial
> +* state retrieved from the hardware during setup.
> +*/
> + return to_intel_crtc(crtc)->active && crtc->fb && crtc->mode.clock;
> +}
> +
> static void i8xx_disable_fbc(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -405,9 +413,8 @@ void intel_update_fbc(struct drm_device *dev)
> * - going to an unsupported config (interlace, pixel multiply,
> etc.)
> */
> list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> - if (tmp_crtc->enabled &&
> - !to_intel_crtc(tmp_crtc)->primary_disabled &&
> - tmp_crtc->fb) {
> + if (intel_crtc_active(tmp_crtc) &&
> + !to_intel_crtc(tmp_crtc)->primary_disabled) {
> if (crtc) {
> DRM_DEBUG_KMS("more than one pipe active,
> disabling compression\n");
> dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
> @@ -992,7 +999,7 @@ static struct drm_crtc *single_enabled_crtc(struct
> drm_device *dev)
> struct drm_crtc *crtc, *enabled = NULL;
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> - if (crtc->enabled && crtc->fb) {
> + if (intel_crtc_active(crtc)) {
> if (enabled)
> return NULL;
> enabled = crtc;
> @@ -1086,7 +1093,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> int entries, tlb_miss;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> - if (crtc->fb == NULL || !crtc->enabled) {
> + if (!intel_crtc_active(crtc)) {
> *cursor_wm = cursor->guard_size;
> *plane_wm = display->guard_size;
> return false;
> @@ -1215,7 +1222,7 @@ static bool vlv_compute_drain_latency(struct drm_device
> *dev,
> int entries;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> - if (crtc->fb == NULL || !crtc->enabled)
> + if (!intel_crtc_active(crtc))
> return false;
>
> clock = crtc->mode.clock; /* VESA DOT Clock */
> @@ -1478,7 +1485,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>
> fifo_size = dev_priv->display.get_fifo_size(dev, 1);
> crtc = intel_get_crtc_for_plane(dev, 1);
> - if (crtc->enabled &&a
Re: [PATCH] drm/i915: disable cpt phase pointer fdi rx workaround
Acked.
-Daniel
On Tue, Jan 15, 2013 at 9:43 AM, CAI Qian wrote:
> Daniel, again this needs your ACK/NAK please. The original patch has
> been modified slightly in order to apply for both 3.4.x and 3.7.x
> stable kernels (does not affect 3.0.x).
>
> Regards,
> CAI Qian
>
> From 539526b4137bc0e7a8806c38c8522f226814a0e6 Mon Sep 17 00:00:00 2001
> From: Daniel Vetter
> Date: Sat, 8 Dec 2012 12:58:33 +0100
> Subject: [PATCH] drm/i915: disable cpt phase pointer fdi rx workaround
>
> We've originally added this in
>
> commit 291427f5fdadec6e4be2924172e8350e1539
> Author: Jesse Barnes
> Date: Fri Jul 29 12:42:37 2011 -0700
>
> drm/i915: apply phase pointer override on SNB+ too
>
> and then copy-pasted it over to ivb/ppt. The w/a was originally added
> for ilk/ibx in
>
> commit 5b2adf897146edeac6a1e438fb67b5a53dbbdf34
> Author: Jesse Barnes
> Date: Thu Oct 7 16:01:15 2010 -0700
>
> drm/i915: add Ironlake clock gating workaround for FDI link training
>
> and fixed up a bit in
>
> commit 6f06ce184c765fd8d50669a8d12fdd566c920859
> Author: Jesse Barnes
> Date: Tue Jan 4 15:09:38 2011 -0800
>
> drm/i915: set phase sync pointer override enable before setting phase
> sync pointer
>
> It turns out that this w/a isn't actually required on cpt/ppt and
> positively harmful on ivb/ppt when using fdi B/C links - it results in
> a black screen occasionally, with seemingfully everything working as
> it should. The only failure indication I've found in the hw is that
> eventually (but not right after the modeset completes) a pipe underrun
> is signalled.
>
> Big thanks to Arthur Runyan for all the ideas for registers to check
> and changes to test, otherwise I couldn't ever have tracked this down!
>
> Cc: "Runyan, Arthur J"
> Cc: stable@vger.kernel.org
> Reviewed-by: Jesse Barnes
> Signed-off-by: Daniel Vetter
> Signed-off-by: CAI Qian
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 08a9b13..4d3c7c6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2302,18 +2302,6 @@ static void intel_fdi_normal_train(struct drm_crtc
> *crtc)
>FDI_FE_ERRC_ENABLE);
> }
>
> -static void cpt_phase_pointer_enable(struct drm_device *dev, int pipe)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 flags = I915_READ(SOUTH_CHICKEN1);
> -
> - flags |= FDI_PHASE_SYNC_OVR(pipe);
> - I915_WRITE(SOUTH_CHICKEN1, flags); /* once to unlock... */
> - flags |= FDI_PHASE_SYNC_EN(pipe);
> - I915_WRITE(SOUTH_CHICKEN1, flags); /* then again to enable */
> - POSTING_READ(SOUTH_CHICKEN1);
> -}
> -
> /* The FDI link training functions for ILK/Ibexpeak. */
> static void ironlake_fdi_link_train(struct drm_crtc *crtc)
> {
> @@ -2464,9 +2452,6 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc)
> POSTING_READ(reg);
> udelay(150);
>
> - if (HAS_PCH_CPT(dev))
> - cpt_phase_pointer_enable(dev, pipe);
> -
> for (i = 0; i < 4; i++) {
> reg = FDI_TX_CTL(pipe);
> temp = I915_READ(reg);
> @@ -2593,9 +2578,6 @@ static void ivb_manual_fdi_link_train(struct drm_crtc
> *crtc)
> POSTING_READ(reg);
> udelay(150);
>
> - if (HAS_PCH_CPT(dev))
> - cpt_phase_pointer_enable(dev, pipe);
> -
> for (i = 0; i < 4; i++) {
> reg = FDI_TX_CTL(pipe);
> temp = I915_READ(reg);
> @@ -2737,17 +2719,6 @@ static void ironlake_fdi_pll_disable(struct intel_crtc
> *intel_crtc)
> udelay(100);
> }
>
> -static void cpt_phase_pointer_disable(struct drm_device *dev, int pipe)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 flags = I915_READ(SOUTH_CHICKEN1);
> -
> - flags &= ~(FDI_PHASE_SYNC_EN(pipe));
> - I915_WRITE(SOUTH_CHICKEN1, flags); /* once to disable... */
> - flags &= ~(FDI_PHASE_SYNC_OVR(pipe));
> - I915_WRITE(SOUTH_CHICKEN1, flags); /* then again to lock */
> - POSTING_READ(SOUTH_CHICKEN1);
> -}
> static void ironlake_fdi_disable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -2777,8 +2748,6 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
> I915_WRITE(FDI_RX_CHICKEN(pipe),
>I915_READ(FDI_RX_CHICKEN(pipe) &
> ~FDI_RX_PHASE_SYNC_POINTER_EN));
> - } else if (HAS_PCH_CPT(dev)) {
> - cpt_phase_pointer_disable(dev, pipe);
> }
>
> /* still set train pattern 1 */
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patch "drm: Add EDID_QUIRK_FORCE_REDUCED_BLANKING for ASUS VW222S" has been added to the 3.4-stable tree
On Tue, Jan 15, 2013 at 3:31 PM, Ilija Hadzic wrote: > I thought I saw a revert for that patch on the mailing list yesterday: > > http://lists.freedesktop.org/archives/dri-devel/2013-January/033322.html Yeah, patch is bogus and the revert is already acked by the original author. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Invalidate the relocation presumed_offsets along the slow path
On Tue, Jan 15, 2013 at 04:17:54PM +, Chris Wilson wrote:
> In the slow path, we are forced to copy the relocations prior to
> acquiring the struct mutex in order to handle pagefaults. We forgo
> copying the new offsets back into the relocation entries in order to
> prevent a recursive locking bug should we trigger a pagefault whilst
> holding the mutex for the reservations of the execbuffer. Therefore, we
> need to reset the presumed_offsets just in case the objects are rebound
> back into their old locations after relocating for this exexbuffer - if
> that were to happen we would assume the relocations were valid and leave
> the actual pointers to the kernels dangling, instant hang.
>
> Fixes regression from commit bcf50e2775bbc3101932d8e4ab8c7902aa4163b4
> Author: Chris Wilson
> Date: Sun Nov 21 22:07:12 2010 +
>
> drm/i915: Handle pagefaults in execbuffer user relocations
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55984
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: stable@vger.kernel.org
Awesome piece of debugging!
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 4532757..40c062d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -767,6 +767,8 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> total = 0;
> for (i = 0; i < count; i++) {
> struct drm_i915_gem_relocation_entry __user *user_relocs;
> + u64 invalid_offset = (u64)-1;
I'm a bit uneasy with the semantics here, fearing that a random piece of
userspace ORs in a few flags instead of adding them. Could we align this
to 4096 bytes? Or maybe enshrine 0 as our official invalid reloc ...
-Daniel
> + int j;
>
> user_relocs = (void __user *)(uintptr_t)exec[i].relocs_ptr;
>
> @@ -777,6 +779,25 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> goto err;
> }
>
> + /* As we do not update the known relocation offsets after
> + * relocating (due to the complexities in lock handling),
> + * we need to mark them as invalid now so that we force the
> + * relocation processing next time. Just in case the target
> + * object is evicted and then rebound into its old
> + * presumed_offset before the next execbuffer - if that
> + * happened we would make the mistake of assuming that the
> + * relocations were valid.
> + */
> + for (j = 0; j < exec[i].relocation_count; j++) {
> + if (copy_to_user(&user_relocs[j].presumed_offset,
> + &invalid_offset,
> + sizeof(invalid_offset))) {
> + ret = -EFAULT;
> + mutex_lock(&dev->struct_mutex);
> + goto err;
> + }
> + }
> +
> reloc_offset[i] = total;
> total += exec[i].relocation_count;
> }
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Invalidate the relocation presumed_offsets along the slow path
On Tue, Jan 15, 2013 at 04:17:54PM +, Chris Wilson wrote:
> In the slow path, we are forced to copy the relocations prior to
> acquiring the struct mutex in order to handle pagefaults. We forgo
> copying the new offsets back into the relocation entries in order to
> prevent a recursive locking bug should we trigger a pagefault whilst
> holding the mutex for the reservations of the execbuffer. Therefore, we
> need to reset the presumed_offsets just in case the objects are rebound
> back into their old locations after relocating for this exexbuffer - if
> that were to happen we would assume the relocations were valid and leave
> the actual pointers to the kernels dangling, instant hang.
>
> Fixes regression from commit bcf50e2775bbc3101932d8e4ab8c7902aa4163b4
> Author: Chris Wilson
> Date: Sun Nov 21 22:07:12 2010 +
>
> drm/i915: Handle pagefaults in execbuffer user relocations
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55984
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: stable@vger.kernel.org
Picked up for -fixes, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 4532757..40c062d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -767,6 +767,8 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> total = 0;
> for (i = 0; i < count; i++) {
> struct drm_i915_gem_relocation_entry __user *user_relocs;
> + u64 invalid_offset = (u64)-1;
> + int j;
>
> user_relocs = (void __user *)(uintptr_t)exec[i].relocs_ptr;
>
> @@ -777,6 +779,25 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
> goto err;
> }
>
> + /* As we do not update the known relocation offsets after
> + * relocating (due to the complexities in lock handling),
> + * we need to mark them as invalid now so that we force the
> + * relocation processing next time. Just in case the target
> + * object is evicted and then rebound into its old
> + * presumed_offset before the next execbuffer - if that
> + * happened we would make the mistake of assuming that the
> + * relocations were valid.
> + */
> + for (j = 0; j < exec[i].relocation_count; j++) {
> + if (copy_to_user(&user_relocs[j].presumed_offset,
> + &invalid_offset,
> + sizeof(invalid_offset))) {
> + ret = -EFAULT;
> + mutex_lock(&dev->struct_mutex);
> + goto err;
> + }
> + }
> +
> reloc_offset[i] = total;
> total += exec[i].relocation_count;
> }
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] iommu/intel: disable DMAR for g4x integrated gfx
DMAR support on g4x/gm45 integrated gpus seems to be totally busted.
So don't bother, but instead disable it by default to allow distros to
unconditionally enable DMAR support.
v2: Actually wire up the right quirk entry, spotted by Adam Jackson.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=51921
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=538163
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=538163
Cc: Adam Jackson
Cc: David Woodhouse
Cc: stable@vger.kernel.org
Tested-by: stathis
Tested-by: Mihai Moldovan
Signed-off-by: Daniel Vetter
---
drivers/iommu/intel-iommu.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d4a4cd4..9743769 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4196,6 +4196,21 @@ static struct iommu_ops intel_iommu_ops = {
.pgsize_bitmap = INTEL_IOMMU_PGSIZES,
};
+static void __devinit quirk_iommu_g4x_gfx(struct pci_dev *dev)
+{
+ /* G4x/GM45 integrated gfx dmar support is totally busted. */
+ printk(KERN_INFO "DMAR: Disabling IOMMU for graphics on this
chipset\n");
+ dmar_map_gfx = 0;
+}
+
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_g4x_gfx);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_g4x_gfx);
+
static void __devinit quirk_iommu_rwbf(struct pci_dev *dev)
{
/*
@@ -4204,12 +4219,6 @@ static void __devinit quirk_iommu_rwbf(struct pci_dev
*dev)
*/
printk(KERN_INFO "DMAR: Forcing write-buffer flush capability\n");
rwbf_quirk = 1;
-
- /* https://bugzilla.redhat.com/show_bug.cgi?id=538163 */
- if (dev->revision == 0x07) {
- printk(KERN_INFO "DMAR: Disabling IOMMU for graphics on this
chipset\n");
- dmar_map_gfx = 0;
- }
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_rwbf);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] intel/iommu: force writebuffer-flush quirk on Gen 4 Chipsets
We already have the quirk entry for the mobile platform, but also
reports on some desktop versions. So be paranoid and set it
everywhere.
References:
http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg33138.html
Cc: stable@vger.kernel.org
Cc: David Woodhouse
Reported-and-tested-by: Mihai Moldovan
Signed-off-by: Daniel Vetter
---
drivers/iommu/intel-iommu.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9743769..19854bf 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4215,13 +4215,19 @@ static void __devinit quirk_iommu_rwbf(struct pci_dev
*dev)
{
/*
* Mobile 4 Series Chipset neglects to set RWBF capability,
-* but needs it:
+* but needs it. Same seems to hold for the desktop versions.
*/
printk(KERN_INFO "DMAR: Forcing write-buffer flush capability\n");
rwbf_quirk = 1;
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_rwbf);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_rwbf);
#define GGC 0x52
#define GGC_MEMORY_SIZE_MASK (0xf << 8)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Disable AsyncFlip performance optimisations
On Mon, Jan 21, 2013 at 10:25:36PM +0200, Imre Deak wrote:
> On Sun, 2013-01-20 at 16:11 +, Chris Wilson wrote:
> > This is a required workarounds for all products, especially on gen6+
> > where it causes the command streamer to fail to parse instructions
> > following a WAIT_FOR_EVENT. We use WAIT_FOR_EVENT for synchronising
> > between the GPU and the display engines, and so this bit being unset may
> > cause hangs.
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=52311
> > Signed-off-by: Chris Wilson
> > Cc: stable@vger.kernel.org
>
> Reviewed-by: Imre Deak
Picked up for -fixes, thanks for the patch.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h |1 +
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 19 +--
> > 2 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8b81052..db1c034 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -536,6 +536,7 @@
> > #define MI_MODE0x0209c
> > # define VS_TIMER_DISPATCH (1 << 6)
> > # define MI_FLUSH_ENABLE (1 << 12)
> > +# define ASYNC_FLIP_PERF_DISABLE (1 << 14)
> >
> > #define GEN6_GT_MODE 0x20d0
> > #define GEN6_GT_MODE_HI (1 << 9)
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index ef68037..af7adb0 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -503,13 +503,20 @@ static int init_render_ring(struct intel_ring_buffer
> > *ring)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > int ret = init_ring_common(ring);
> >
> > - if (INTEL_INFO(dev)->gen > 3) {
> > + if (INTEL_INFO(dev)->gen > 3)
> > I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
> > - if (IS_GEN7(dev))
> > - I915_WRITE(GFX_MODE_GEN7,
> > -
> > _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
> > - _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
> > - }
> > +
> > + /* We need to disable the AsyncFlip performance optimisations in order
> > +* to use MI_WAIT_FOR_EVENT within the CS. It should already be
> > +* programmed to '1' on all products.
> > +*/
> > + if (INTEL_INFO(dev)->gen >= 6)
> > + I915_WRITE(MI_MODE,
> > _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
> > +
> > + if (IS_GEN7(dev))
> > + I915_WRITE(GFX_MODE_GEN7,
> > + _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
> > + _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
> >
> > if (INTEL_INFO(dev)->gen >= 5) {
> > ret = init_pipe_control(ring);
>
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: GFX_MODE Flush TLB Invalidate Mode must be '1' for scanline waits
On Sun, Jan 20, 2013 at 07:36:39PM -0800, Ben Widawsky wrote: > On Sun, Jan 20, 2013 at 04:33:32PM +, Chris Wilson wrote: > > On SNB, if bit 13 of GFX_MODE, Flush TLB Invalidate Mode, is not set to 1, > > the hardware can not program the scanline values. Those scanline values > > then control when the signal is sent from the display engine to the render > > ring for MI_WAIT_FOR_EVENTs. Note setting this bit means that TLB > > invalidations must be performed explicitly through the appropriate bits > > being set in PIPE_CONTROL. > > > > References: https://bugzilla.kernel.org/show_bug.cgi?id=52311 > > Signed-off-by: Chris Wilson > > Cc: stable@vger.kernel.org > > This all sounds a bit hand-wavy to me. First, it's not clear from the > commit message if this actually fixes anything. Is that connection > between bit 13 of GFX_MODE and the scanline updates documented, or was > it just "found." If it was found we should get a doc update, or > clarification, because I can't find that in the docs, and perhaps more > importantly, I can't even come up with a theory why the TLB would have > any effect on the problem. > > OTOH, I've always disliked that this bit wasn't explicitly set. As a > note there, we have a context workaround you could update as a result of > this patch. > > My only real concern is over old userspace with this patch. Were there > any released ddx, or mesa which didn't set the bit on the PIPE_CONTROL? > Does anyone care? It'd be nice if we had some way to observe the TLBs > weren't being invalidated as needed. > > If you can address my concerns over breaking old userspace, and don't > feel like addressing my other comments, it is (and the next patch): > Reviewed-by: Ben Widawsky Picked up for -fixes, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/intel: disable DMAR for g4x integrated gfx
On Mon, Jan 21, 2013 at 01:03:48PM -0600, David Woodhouse wrote: > On Sun, 2013-01-20 at 23:50 +0100, Daniel Vetter wrote: > > DMAR support on g4x/gm45 integrated gpus seems to be totally busted. > > So don't bother, but instead disable it by default to allow distros to > > unconditionally enable DMAR support. > > Acked-By: David Woodhouse Ok, I've picked that up into my drm-intel-fixes tree and will send it off to Dave in the next few days. > It *really* winds me up that we never bother to test this hardware > before we ship it. > > But I'm even *more* disappointed that we can't even diagnose it and > publish coherent errata *after* the fact. I'd really like to see each > quirk which disables features referencing a specific published erratum. > We really ought to be able to manage at least *that* much. > > Rajesh? Yeah, some real quirk notice would be nice. I've hunted down the gen4 errata sheets, but there's nothing in there about the gfx not working for dmar. Hence I'm opting for a working gpu in case of doubts. Also note that according to intel docs only the gm45 and g45 have vt-d support. So with this bug report we have them all covered. I've still left all the other gen4 ids in the quirk tables, just in case intel marketing materials win another round against me. Instead amended the commit message a bit. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: fix FORCEWAKE posting reads
On Wed, Jan 23, 2013 at 7:15 AM, CAI Qian wrote:
> Hello Daniel,
>
> Could you please help ACK/NAK this back-port patch for the stable-3.7.y?
> It fixed up the context and dropped the modification for
> vlv_force_wake_reset() due to it was only introduced later.
Yeah, looks good.
-Daniel
>
> Regards,
> CAI Qian
>
> From b514407547890686572606c9dfa4b7f832db9958 Mon Sep 17 00:00:00 2001
> From: Jani Nikula
> Date: Thu, 17 Jan 2013 10:24:09 +0200
> Subject: [PATCH] drm/i915: fix FORCEWAKE posting reads
>
> We stopped reading FORCEWAKE for posting reads in
>
> commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
> Author: Ben Widawsky
> Date: Sat Sep 1 22:59:50 2012 -0700
>
> drm/i915: Never read FORCEWAKE
>
> and started using something from the same cacheline instead. On the
> bug reporter's machine this broke entering rc6 states after a
> suspend/resume cycle. It turns out reading ECOBUS as posting read
> worked fine, while GTFIFODBG did not, preventing RC6 states after
> suspend/resume per the bug report referenced below. It's not entirely
> clear why, but clearly GTFIFODBG was nowhere near the same cacheline
> or address range as FORCEWAKE.
>
> Trying out various registers for posting reads showed that all tested
> registers for which NEEDS_FORCE_WAKE() (in i915_drv.c) returns true
> work. Conversely, most (but not quite all) registers for which
> NEEDS_FORCE_WAKE() returns false do not work. Details in the referenced
> bug.
>
> Based on the above, add posting reads on ECOBUS where GTFIFODBG was
> previously relied on.
>
> In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as
> well, but instead of ECOBUS, use FORCEWAKE_ACK_VLV which is in the same
> address range as FORCEWAKE_VLV.
>
> v2: Add more details to the commit message. No functional changes.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411
> Reported-and-tested-by: Alexander Bersenev
> CC: Ben Widawsky
> Signed-off-by: Jani Nikula
> Reviewed-by: Chris Wilson
> Cc: stable@vger.kernel.org
> [danvet: add cc: stable and make the commit message a bit clearer that
> this is a regression fix and what exactly broke.]
> Signed-off-by: Daniel Vetter
> Signed-off-by: CAI Qian
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index eaaff3c..a01ab2a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4014,7 +4014,8 @@ static void __gen6_gt_force_wake_get(struct
> drm_i915_private *dev_priv)
> DRM_ERROR("Timed out waiting for forcewake old ack to
> clear.\n");
>
> I915_WRITE_NOTRACE(FORCEWAKE, 1);
> -POSTING_READ(ECOBUS); /* something from same cacheline, but
> !FORCEWAKE */
> +/* something from same cacheline, but !FORCEWAKE */
> +POSTING_READ(ECOBUS);
>
> if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
> FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -4037,7 +4038,8 @@ static void __gen6_gt_force_wake_mt_get(struct
> drm_i915_private *dev_priv)
> DRM_ERROR("Timed out waiting for forcewake old ack to
> clear.\n");
>
> I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> -POSTING_READ(ECOBUS); /* something from same cacheline, but
> !FORCEWAKE */
> +/* something from same cacheline, but !FORCEWAKE */
> +POSTING_READ(ECOBUS);
>
> if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
> FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -4074,14 +4076,16 @@ void gen6_gt_check_fifodbg(struct drm_i915_private
> *dev_priv)
> static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> {
> I915_WRITE_NOTRACE(FORCEWAKE, 0);
> -/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
> +/* something from same cacheline, but !FORCEWAKE */
> +POSTING_READ(ECOBUS);
> gen6_gt_check_fifodbg(dev_priv);
> }
>
> static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
> {
> I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(1));
> -/* gen6_gt_check_fifodbg doubles as the POSTING_READ */
> +/* something from same cacheline, but !FORCEWAKE_MT */
> +POSTING_READ(ECOBUS);
> gen6_gt_check_fifodbg(dev_priv);
> }
>
> @@ -4136,7 +4140,8 @@ static void vlv_force_wake_get(struct drm_i915_private
> *dev_priv)
> static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> {
> I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(1));
> -/* The below doubles as a POSTING
stable backport request
Dear stable team Please backport the following patch to all stable kernels: 4283908ef7f11a72c3b80dd4cf026f1a86429f82 drm/i915: Implement WaDisableHiZPlanesWhenMSAAEnabled Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50545 Tested-by: Wen-chien Jesse Sung Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert "drm: Add EDID_QUIRK_FORCE_REDUCED_BLANKING for ASUS VW222S"
On Mon, Jan 14, 2013 at 3:37 PM, Paul Menzel wrote: > Am Montag, den 14.01.2013, 11:06 +0100 schrieb Daniel Vetter: >> This reverts commit 6f33814bd4d9cfe76033a31b1c0c76c960cd8e4b. >> >> The quirk cause a regression, and it looks like the original bug was >> simply a lack of FIFO bandwidth on the i915G of the reporter. Which >> should eventually be fixed as soon as we get around to implemented >> DSPARB FIFO reassignment on gen 3. > > Reported-by: Florian Mickler > >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52281 >> Cc: stable@vger.kernel.org >> Signed-off-by: Daniel Vetter > > Acked-by: Paul Menzel Hm, revert hasn't landed anywhere yet :( Dave, Ajax: ping? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -stable 3.7] iommu/intel: disable DMAR for g4x integrated gfx
On Wed, Jan 30, 2013 at 5:11 PM, Mihai Moldovan wrote:
> REQ ACK. :)
Looks good.
-Daniel
> >From 9452618e7462181ed9755236803b6719298a13ce Mon Sep 17 00:00:00 2001
> From: Daniel Vetter
> Date: Sun, 20 Jan 2013 23:50:13 +0100
> Subject: [PATCH] iommu/intel: disable DMAR for g4x integrated gfx
>
> DMAR support on g4x/gm45 integrated gpus seems to be totally busted.
> So don't bother, but instead disable it by default to allow distros to
> unconditionally enable DMAR support.
>
> v2: Actually wire up the right quirk entry, spotted by Adam Jackson.
>
> Note that according to intel marketing materials only g45 and gm45
> support DMAR/VT-d. So we have reports for all relevant gen4 pci ids by
> now. Still, keep all the other gen4 ids in the quirk table in case the
> marketing stuff confused me again, which would not be the first time.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=51921
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=538163
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=538163
> Cc: Adam Jackson
> Cc: David Woodhouse
> Cc: stable@vger.kernel.org
> Tested-by: stathis
> Tested-by: Mihai Moldovan
> Signed-off-by: Daniel Vetter
> Signed-off-by: Mihai Moldovan
> ---
> drivers/iommu/intel-iommu.c | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index c2c07a4..be3d7dd 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4234,6 +4234,21 @@ static struct iommu_ops intel_iommu_ops = {
> .pgsize_bitmap = INTEL_IOMMU_PGSIZES,
> };
>
> +static void __devinit quirk_iommu_g4x_gfx(struct pci_dev *dev)
> +{
> + /* G4x/GM45 integrated gfx dmar support is totally busted. */
> + printk(KERN_INFO "DMAR: Disabling IOMMU for graphics on this
> chipset\n");
> + dmar_map_gfx = 0;
> +}
> +
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_g4x_gfx);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_g4x_gfx);
> +
> static void __devinit quirk_iommu_rwbf(struct pci_dev *dev)
> {
> /*
> @@ -4242,12 +4257,6 @@ static void __devinit quirk_iommu_rwbf(struct pci_dev
> *dev)
> */
> printk(KERN_INFO "DMAR: Forcing write-buffer flush capability\n");
> rwbf_quirk = 1;
> -
> - /* https://bugzilla.redhat.com/show_bug.cgi?id=538163 */
> - if (dev->revision == 0x07) {
> - printk(KERN_INFO "DMAR: Disabling IOMMU for graphics on this
> chipset\n");
> - dmar_map_gfx = 0;
> - }
> }
>
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_rwbf);
> --
> 1.8.1
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Only run idle processing from i915_gem_retire_requests_worker
On Wed, Jan 30, 2013 at 03:32:33PM -0200, Rodrigo Vivi wrote: > Reviewed-by: Rodrigo Vivi > > > On Mon, Jan 28, 2013 at 9:02 AM, Chris Wilson > wrote: > > On Tue, Jan 08, 2013 at 11:02:57AM +, Chris Wilson wrote: > >> When adding the fb idle detection to mark-inactive, it was forgotten > >> that userspace can drive the processing of retire-requests. We assumed > >> that it would be principally driven by the retire requests worker, > >> running once every second whilst active and so we would get the deferred > >> timer for free. Instead we spend too many CPU cycles reclocking the LVDS > >> preventing real work from being done. > >> > >> Signed-off-by: Chris Wilson > >> Reported-and-tested-by: Alexander Lam > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58843 > >> Cc: stable@vger.kernel.org > > > > Poke. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Restore GTT domain tracking of unbound objects upon resume
On Wed, Feb 06, 2013 at 09:31:20AM +, Chris Wilson wrote: > In order for the objects to be coherent in uncached memory (as they are > presumed to be on the unbound list) we need to clflush them because > experience dictates that the CPU caches will be dirtied across > hibernation. > > Cc: stable@vger.kernel.org > Signed-off-by: Chris Wilson I think this change will freak out Jesse, since it'll kill resume time when we clflush a few GB of memory. So could you throw another patch on top of this series (maybe after the unbound dropping) which calls restore_gtt only when thawing from hibernate, but not when resuming? > --- > drivers/gpu/drm/i915/i915_gem_gtt.c |3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index bdaca3f..368c821 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -404,6 +404,9 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) > i915_gem_gtt_bind_object(obj, obj->cache_level); > } > > + list_for_each_entry(obj, &dev_priv->mm.unbound_list, gtt_list) > + i915_gem_clflush_object(obj); > + > i915_gem_chipset_flush(dev); > } > > -- > 1.7.10.4 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: write backlight harder
770c12312ad617172b1a65b911d3e6564fc5aca8 is the first bad commit
commit 770c12312ad617172b1a65b911d3e6564fc5aca8
Author: Takashi Iwai
Date: Sat Aug 11 08:56:42 2012 +0200
drm/i915: Fix blank panel at reopening lid
changed the register write sequence for restoring the backlight, which
helped prevent non-working backlights on some machines. Turns out that
the original sequence was the right thing to do for a different set of
machines. Worse, setting the backlight level _after_ enabling it seems
to reset it somehow. So we need to make that one conditional upon the
backlight having been reset to zero, and add the old one back.
Cargo-culting at it's best, but it seems to work.
Cc: stable@vger.kernel.org
Cc: Takashi Iwai
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=47941
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_panel.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index bee8cb6..a3730e0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -321,6 +321,9 @@ void intel_panel_enable_backlight(struct drm_device *dev,
if (dev_priv->backlight_level == 0)
dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
+ dev_priv->backlight_enabled = true;
+ intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+
if (INTEL_INFO(dev)->gen >= 4) {
uint32_t reg, tmp;
@@ -356,12 +359,12 @@ void intel_panel_enable_backlight(struct drm_device *dev,
}
set_level:
- /* Call below after setting BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1.
-* BLC_PWM_CPU_CTL may be cleared to zero automatically when these
-* registers are set.
+ /* Check the current backlight level and try to set again if it's zero.
+* On some machines, BLC_PWM_CPU_CTL is cleared to zero automatically
+* when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 are written.
*/
- dev_priv->backlight_enabled = true;
- intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+ if (!intel_panel_get_backlight(dev))
+ intel_panel_actually_set_backlight(dev,
dev_priv->backlight_level);
}
static void intel_panel_init_backlight(struct drm_device *dev)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: write backlight harder
On Wed, Feb 6, 2013 at 1:29 PM, Jani Nikula wrote: > Given how much pain the backlight keeps giving us, do you think we would > benefit from sticking a DRM_DEBUG_DRIVER in the if block there? > > Either way, with a heavy *sigh*, > Reviewed-by: Jani Nikula Merged to dinq, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 1/2] drm/i915: disable shared panel fitter for pipe
On Fri, Feb 8, 2013 at 3:35 PM, Mika Kuoppala
wrote:
> If encoder is switched off by BIOS, but the panel fitter is left on,
> we never try to turn off the panel fitter and leave it still attached
> to the pipe - which can cause blurry output elsewhere.
>
> Based on work by Chris Wilson
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58867
Tested-by from the reporter is missing.
> Cc: stable@vger.kernel.org
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_display.c |9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..9b5f0fb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3656,6 +3656,15 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>
> intel_disable_plane(dev_priv, plane, pipe);
> intel_disable_pipe(dev_priv, pipe);
> +
> + if (!HAS_PCH_SPLIT(dev)) {
This is always true in i9xx_crtc_disable, so you can remove this indentation.
-Daniel
> + const u32 pctl = I915_READ(PFIT_CONTROL);
> +
> + if ((pctl & PFIT_ENABLE) &&
> + ((pctl & PFIT_PIPE_MASK) >> PFIT_PIPE_SHIFT) == pipe)
> + I915_WRITE(PFIT_CONTROL, 0);
> + }
> +
> intel_disable_pll(dev_priv, pipe);
>
> intel_crtc->active = false;
> --
> 1.7.9.5
>
> _______
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] intel/iommu: force writebuffer-flush quirk on Gen 4 Chipsets
On Tue, Jan 22, 2013 at 7:55 PM, David Woodhouse wrote:
> On Mon, 2013-01-21 at 19:48 +0100, Daniel Vetter wrote:
>> We already have the quirk entry for the mobile platform, but also
>> reports on some desktop versions. So be paranoid and set it
>> everywhere.
>>
>> References:
>> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg33138.html
>> Cc: stable@vger.kernel.org
>> Cc: David Woodhouse
>> Reported-and-tested-by: Mihai Moldovan
>> Signed-off-by: Daniel Vetter
>> ---
>> drivers/iommu/intel-iommu.c |8 +++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 9743769..19854bf 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -4215,13 +4215,19 @@ static void __devinit quirk_iommu_rwbf(struct
>> pci_dev *dev)
>> {
>> /*
>>* Mobile 4 Series Chipset neglects to set RWBF capability,
>> - * but needs it:
>> + * but needs it. Same seems to hold for the desktop versions.
>>*/
>> printk(KERN_INFO "DMAR: Forcing write-buffer flush capability\n");
>> rwbf_quirk = 1;
>> }
>>
>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_rwbf);
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_rwbf);
>>
>> #define GGC 0x52
>> #define GGC_MEMORY_SIZE_MASK (0xf << 8)
>
> Again, I'm really unhappy about doing this kind of thing based on
> hearsay. This should have a specific reference (with URL) to a published
> erratum. Rajesh?
Any updates here? I'd like to include this for 3.9 -next with cc:
stable to finally allow distros to enable DMAR/IOMMU support by
default on Intel hw. The current state of affairs is simply
embarrassing imo :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: add missing \n to UTS_RELEASE in the error_state
On Thu, Feb 14, 2013 at 10:02:12AM +, Chris Wilson wrote: > On Thu, Feb 14, 2013 at 11:23:35AM +0200, Jani Nikula wrote: > > Amending > > commit 4518f611ba21ba165ea3714055938a8984a44ff9 > > Author: Daniel Vetter > > Date: Wed Jan 23 16:16:35 2013 +0100 > > > > drm/i915: dump UTS_RELEASE into the error_state > > Perhaps mention which commit introduced UTS_RELEASE, though I thnk the > any stable conflict will be remarkably easy to resolve. > > > CC: stable@vger.kernel.org > > Signed-off-by: Jani Nikula > > And I just got used to the enhanced PCI ID string. > Reviewed-by: Chris Wilson Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Use HAS_L3_GPU_CACHE in i915_gem_l3_remap
Yet another remnant ... this might explain why l3 remapping didn't really work on HSW. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57441 Spotted-by: Ville Syrjälä Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a367801..8413ffc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3872,7 +3872,7 @@ void i915_gem_l3_remap(struct drm_device *dev) u32 misccpctl; int i; - if (!IS_IVYBRIDGE(dev)) + if (!HAS_L3_GPU_CACHE(dev)) return; if (!dev_priv->l3_parity.remap_info) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Use HAS_L3_GPU_CACHE in i915_gem_l3_remap
On Thu, Feb 14, 2013 at 02:51:19PM -0800, Ben Widawsky wrote: > On Thu, Feb 14, 2013 at 07:46:07PM +0100, Daniel Vetter wrote: > > Yet another remnant ... this might explain why l3 remapping didn't > > really work on HSW. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57441 > > Spotted-by: Ville Syrjälä > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter > Reviewed-by: Ben Widawsky Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: inverted brightness quirk for Acer Aspire 4736Z
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=53881
Cc: stable@vger.kernel.org
Cc: Jani Nikula
Tested-by: Jani Monoses
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 593c668..c9808e8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8679,6 +8679,9 @@ static struct intel_quirk intel_quirks[] = {
/* Acer/Packard Bell NCL20 */
{ 0x2a42, 0x1025, 0x034b, quirk_invert_brightness },
+
+ /* Acer Aspire 4736Z */
+ { 0x2a42, 0x1025, 0x0260, quirk_invert_brightness },
};
static void intel_init_quirks(struct drm_device *dev)
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm/i915: inverted brightness quirk for Acer Aspire 4736Z
On Fri, Feb 15, 2013 at 06:35:30PM +0100, Daniel Vetter wrote:
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=53881
> Cc: stable@vger.kernel.org
> Cc: Jani Nikula
> Tested-by: Jani Monoses
> Signed-off-by: Daniel Vetter
And merged right away ...
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 593c668..c9808e8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8679,6 +8679,9 @@ static struct intel_quirk intel_quirks[] = {
>
> /* Acer/Packard Bell NCL20 */
> { 0x2a42, 0x1025, 0x034b, quirk_invert_brightness },
> +
> + /* Acer Aspire 4736Z */
> + { 0x2a42, 0x1025, 0x0260, quirk_invert_brightness },
> };
>
> static void intel_init_quirks(struct drm_device *dev)
> --
> 1.7.10.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Disable WC PTE updates to w/a buggy IOMMU on ILK
On Wed, Feb 13, 2013 at 09:31:53AM +, Chris Wilson wrote: > Whilst IOMMU is enabled for the Intel GPU on Ironlake, it appears that > using WC writes to update the PTE on the GPU fail miserably. The result > looks like the majority of the writes do not land leading to lots of > screen corruption and a hard system hang. > > v2: s/ > Reported-by: Nathan Myers > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=60391 > Signed-off-by: Chris Wilson > Cc: stable@vger.kernel.org Queued for -next with cc: stable removed, since the regression is only in -next. Thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] intel/iommu: force writebuffer-flush quirk on Gen 4 Chipsets
On Wed, Feb 13, 2013 at 07:40:05PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2013 at 7:55 PM, David Woodhouse wrote:
> > On Mon, 2013-01-21 at 19:48 +0100, Daniel Vetter wrote:
> >> We already have the quirk entry for the mobile platform, but also
> >> reports on some desktop versions. So be paranoid and set it
> >> everywhere.
> >>
> >> References:
> >> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg33138.html
> >> Cc: stable@vger.kernel.org
> >> Cc: David Woodhouse
> >> Reported-and-tested-by: Mihai Moldovan
> >> Signed-off-by: Daniel Vetter
> >> ---
> >> drivers/iommu/intel-iommu.c |8 +++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> >> index 9743769..19854bf 100644
> >> --- a/drivers/iommu/intel-iommu.c
> >> +++ b/drivers/iommu/intel-iommu.c
> >> @@ -4215,13 +4215,19 @@ static void __devinit quirk_iommu_rwbf(struct
> >> pci_dev *dev)
> >> {
> >> /*
> >>* Mobile 4 Series Chipset neglects to set RWBF capability,
> >> - * but needs it:
> >> + * but needs it. Same seems to hold for the desktop versions.
> >>*/
> >> printk(KERN_INFO "DMAR: Forcing write-buffer flush capability\n");
> >> rwbf_quirk = 1;
> >> }
> >>
> >> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e10, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e20, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e30, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e40, quirk_iommu_rwbf);
> >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e90, quirk_iommu_rwbf);
> >>
> >> #define GGC 0x52
> >> #define GGC_MEMORY_SIZE_MASK (0xf << 8)
> >
> > Again, I'm really unhappy about doing this kind of thing based on
> > hearsay. This should have a specific reference (with URL) to a published
> > erratum. Rajesh?
>
> Any updates here? I'd like to include this for 3.9 -next with cc:
> stable to finally allow distros to enable DMAR/IOMMU support by
> default on Intel hw. The current state of affairs is simply
> embarrassing imo :(
Whatever, I want to get this off my table, so merged to drm-intel-fixes
for 3.9.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Handle untiled planes when computing their offsets
On Thu, Feb 21, 2013 at 10:17:37PM +0200, Ville Syrjälä wrote: > On Thu, Feb 21, 2013 at 08:04:31PM +, Chris Wilson wrote: > > We trim the fb to fit the CRTC by computing the offset of that CRTC to > > its nearest tile_row origin. This allows us to use framebuffers that are > > larger than the CRTC limits without additional work. > > > > However, we failed to compute the offset for a linear framebuffer > > correctly as we treated its x-advance in whole tiles (instead of the > > linear increment expected), leaving the CRTC misaligned with its > > contents. > > > > Fixes regression from commit c2c75131244507c93f812862fdbd4f3a37139401 > > Author: Daniel Vetter > > Date: Thu Jul 5 12:17:30 2012 +0200 > > > > drm/i915: adjust framebuffer base address on gen4+ > > > > v2: Adjust relative x-coordinate after linear alignment (vsyrjala) > > v3: Repaint with pokadots (vsyrjala) > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61152 > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: stable@vger.kernel.org > > Looking good. > > Reviewed-by: Ville Syrjälä Picked up for -fixes, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
drm/i915 stable backports
Dear stable team, Please backport commit bc86625a4ff7574d4d4dba79723457711eb784e0 Author: Chris Wilson Date: Sun Jul 21 16:00:03 2013 +0100 drm/i915: Retry DP aux_ch communications with a different clock after failure to stable kernels 3.10 and 3.11. It fixes a regression introduced in commit 2c55c336a71cb32ae837dc829d216dc86ed9d84f Author: Jani Nikula Date: Tue Apr 9 08:11:00 2013 +0300 drm/i915: use lower aux clock divider on non-ULT HSW See https://bugs.freedesktop.org/show_bug.cgi?id=70301 Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3.10][v3.11][v3.12][Regression][PATCH 1/1] Revert "Revert "drm/i915: revert eDP bpp clamping code changes""
On Wed, Oct 16, 2013 at 04:34:57PM -0400, Joseph Salisbury wrote: > BugLink: http://bugs.launchpad.net/bugs/1195483 > > This reverts commit 657445fe8660100ad174600ebfa61536392b7624. > > Signed-off-by: Joseph Salisbury > Cc: Daniel Vetter > Cc: Paulo Zanoni > Cc: David Airlie > Cc: stable@vger.kernel.org It's by far not that simple. Jani is working on both the underlying bug and a better w/a. See https://bugzilla.kernel.org/show_bug.cgi?id=59841 for the full story in its entire glory. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drm: allocate crtc mutex separately from crtc
On Thu, Oct 17, 2013 at 1:35 AM, Ilija Hadzic wrote: > Embedding a mutex inside drm_crtc structure evokes a > subtle and rare corruption in drm_crtc_helper_set_config > failure-recovery path. > > Namely, if drm_crtc_helper_set_config takes the > path under 'fail:' label *and* some other process > has attempted to grab the crtc mutex (and got blocked), > restoring the CRTC structure (which is done by bit-copying > the entire structure from saved_crtcs array) will overwrite > the mutex state and the waiters list pointer within the mutex > structure. Consequently the blocked process will never > be scheduled. > > This patch fixes the issue by un-embeding the mutex structure > and allocating it separately from the drm_crtc structure > when the CRTC is initialized. The bit-copying in > drm_crtc_helper_set_config helper will now overwrite the pointer > which is never modified during the CRTC's lifetime, thus > avoiding corruption. > > Signed-off-by: Ilija Hadzic > Cc: stable@vger.kernel.org Can't we instead just copy the few things we need to restore back? This wholesale structure copying has rather tricky semantics and is bound to trick up someone else in the future. And indeed we seem to have a similar (but less catastrophic) thing going on with crtc->fb I think. I've looked a bit through the code and I think we don't actually need to restore anything for crtcs. We pass the mode, fb and offsets in explicitly, and everything else in drm_crtc is derived state. This is also the same that i915 does, we only restore the connector/encoder links even. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drm/i915 stable backports
On Wed, Oct 16, 2013 at 08:42:41AM -0700, Greg KH wrote:
> On Wed, Oct 16, 2013 at 10:28:51AM +0200, Daniel Vetter wrote:
> > Dear stable team,
> >
> > Please backport
> >
> > commit bc86625a4ff7574d4d4dba79723457711eb784e0
> > Author: Chris Wilson
> > Date: Sun Jul 21 16:00:03 2013 +0100
> >
> > drm/i915: Retry DP aux_ch communications with a different clock
> > after failure
> >
> > to stable kernels 3.10 and 3.11. It fixes a regression introduced in
>
> I would, but it fails to apply at all:
> patching file drivers/gpu/drm/i915/intel_dp.c
> Hunk #1 FAILED at 276.
> Hunk #2 FAILED at 290.
> Hunk #3 FAILED at 319.
> Hunk #4 succeeded at 345 (offset -8 lines).
> Hunk #5 FAILED at 1457.
> 4 out of 5 hunks FAILED -- rejects in file drivers/gpu/drm/i915/intel_dp.c
>
> Care to provide a backport?
It needs b84a1cf8950ed075c4ab2630514d4caaae504176 from upstream as a
prerequisite (applies cleanly here on latest 3.11.x) and then the patch
applies with a bit of wiggling. Rebased patch below.
Thanks, Daniel
---
>From 1d1dd21bc86e38020a51b0a9a5d60b36263c820d Mon Sep 17 00:00:00 2001
From: Chris Wilson
Date: Sun, 21 Jul 2013 16:00:03 +0100
Subject: [PATCH] drm/i915: Retry DP aux_ch communications with a different
clock after failure
This is bc86625a4ff7574d4d4dba79723457 upstream.
The w/a db makes the recommendation to both use a non-default value for
the initial clock and then to retry with an alternative clock for
Haswell with the Lakeport PCH.
"On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
failure, retry at least three times with 63, then retry at least three
times with 72 decimal (048h)."
Signed-off-by: Chris Wilson
Reviewed-by: Jani Nikula
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_dp.c | 90 +++--
1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ef9d7f0..3a0f3a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -276,7 +276,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool
has_aux_irq)
return status;
}
-static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
+static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
+ int index)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -290,22 +291,27 @@ static uint32_t get_aux_clock_divider(struct intel_dp
*intel_dp)
* clock divider.
*/
if (IS_VALLEYVIEW(dev)) {
- return 100;
+ return index ? 0 : 100;
} else if (intel_dig_port->port == PORT_A) {
+ if (index)
+ return 0;
if (HAS_DDI(dev))
- return DIV_ROUND_CLOSEST(
- intel_ddi_get_cdclk_freq(dev_priv), 2000);
+ return
DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
else if (IS_GEN6(dev) || IS_GEN7(dev))
return 200; /* SNB & IVB eDP input clock at 400Mhz */
else
return 225; /* eDP input clock at 450Mhz */
} else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
/* Workaround for non-ULT HSW */
- return 74;
+ switch (index) {
+ case 0: return 63;
+ case 1: return 72;
+ default: return 0;
+ }
} else if (HAS_PCH_SPLIT(dev)) {
- return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+ return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
} else {
- return intel_hrawclk(dev) / 2;
+ return index ? 0 :intel_hrawclk(dev) / 2;
}
}
@@ -319,10 +325,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
uint32_t ch_data = ch_ctl + 4;
+ uint32_t aux_clock_divider;
int i, ret, recv_bytes;
uint32_t status;
- uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
- int try, precharge;
+ int try, precharge, clock = 0;
bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
/* dp aux is extremely sensitive to irq latency, hence request the
@@ -353,37 +359,41 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
goto out;
}
- /* Must try at least 3 times according to DP spec */
- for (try = 0; try < 5; try++) {
- /* Load the send data into the aux channel data registers */
-
[PATCH] drm/i915: Sanitize the PPT fdi lane bifurcate state on ivb
We expect this bit to be always set when possible, but some BIOSes are
lazy and don't do this. The result is a pile of WARNs and unhappy fdi
link training code ...
v2: It's actually the inverse: The BIOS sets this bit when it's not
strictly needed. This should be cleaned up in the
global_modeset_resources callback, but we've failed to look at the
active bit. Which means this won't fire (and so clean up BIOS state)
when enabling pipe B or C for the first time.
v3: Wrap lines.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
Tested-by: Jan-Michael Brummer
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cfe9e709..3569db6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
FDI_FE_ERRC_ENABLE);
}
-static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return intel_crtc->base.enabled && intel_crtc->config.has_pch_encoder;
+ return crtc->base.enabled && crtc->active &&
+ crtc->config.has_pch_encoder;
}
static void ivb_modeset_global_resources(struct drm_device *dev)
--
1.8.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the PPT fdi lane bifurcate state on ivb
On Wed, Oct 23, 2013 at 12:58 PM, Ville Syrjälä wrote: > I'm thinking the crtc->base.enabled check is actually pointless. > AFAICS we should never get here with crtc->base.enabled==false and > crtc->active==true Hm yeah. I was kinda shooting for the minimal thing here. > We anyway re-enable the bifurcation bit when we do the mode_set. > Actually that in itself could be a maybe a bug. We'd turn off the > bifurcation bit when both pipes B and C are disabled based on > prepare_pipes, but we only do the mode_set based on modeset_pipes. > But currently we are saved by the fact that those two bitmasks are > the same. The current logic (after this patch) is to - always clear the bit when both pipes are off. - always set if if we either enable pipe B or C and we'd need it to allow the other pipe to be lit up. If pipe B uses 4 lanes then we can't light pipe C up anymore, so we don't set the bit. There is a potential issue though with all pipes disabled with dpms off now. I think conceptually I need to precompute the desired state of the bifurcate bit in the compute config stage and then only set it here. > Another potential bug I found is that we always set the bifurcation > bit in mode_set, even if we're not using FDI. So if we have eg. > pipe B enabled w/ FDI using 4 lanes, and then we enable pipe C w/ EDP, > we'd still flip the bifurcation bit in pipe C's mode_set and destroy pipe > B's output. The fix would be to call ivybridge_update_fdi_bc_bifurcation() > only when has_pch_encoder==true. has_pch_encoder should only be true if we have fdi_lanes > 0. So we shouldn't actually enable this stuff (and iirc I've tested the edp on pipe C with pipe B using 4 lanes). The actual configuration checks all happen in the pipe configuration computation stage. I'll rework the patch a bit (but that'll take a while). Thanks for the review, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb
Originally I've thought that this is leftover hw state dirt from the
BIOS. But after way too much helpless flailing around on my part I've
noticed that the actual bug is when we change the state of an already
active pipe.
For example when we change the fdi lines from 2 to 3 without switching
off outputs in-between we'll never see the crucial on->off transition
in the ->modeset_global_resources hook the current logic relies on.
Patch version 2 got this right by instead also checking whether the
pipe is indeed active. But that in turn broke things when pipes have
been turned off through dpms since the bifurcate enabling is done in
the ->crtc_mode_set callback.
To address this issues discussed with Ville in the patch review move
the setting of the bifurcate bit into the ->crtc_enable hook. That way
we won't wreak havoc with this state when userspace puts all other
outputs into dpms off state. This also moves us forward with our
overall goal to unify the modeset and dpms on paths (which we need to
have to allow runtime pm in the dpms off state).
Unfortunately this requires us to move the bifurcate helpers around a
bit.
Also update the commit message, I've misanalyzed the bug rather badly.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
Tested-by: Jan-Michael Brummer
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 95 ++--
1 file changed, 48 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8c3bf8a89cb7..509762c85d2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
FDI_FE_ERRC_ENABLE);
}
-static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
+static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return intel_crtc->base.enabled && intel_crtc->config.has_pch_encoder;
+ return crtc->base.enabled && crtc->active &&
+ crtc->config.has_pch_encoder;
}
static void ivb_modeset_global_resources(struct drm_device *dev)
@@ -3074,6 +3075,48 @@ static void ironlake_pch_transcoder_set_timings(struct
intel_crtc *crtc,
I915_READ(VSYNCSHIFT(cpu_transcoder)));
}
+static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ uint32_t temp;
+
+ temp = I915_READ(SOUTH_CHICKEN1);
+ if (temp & FDI_BC_BIFURCATION_SELECT)
+ return;
+
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
+ WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
+
+ temp |= FDI_BC_BIFURCATION_SELECT;
+ DRM_DEBUG_KMS("enabling fdi C rx\n");
+ I915_WRITE(SOUTH_CHICKEN1, temp);
+ POSTING_READ(SOUTH_CHICKEN1);
+}
+
+static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
+{
+ struct drm_device *dev = intel_crtc->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ switch (intel_crtc->pipe) {
+ case PIPE_A:
+ break;
+ case PIPE_B:
+ if (intel_crtc->config.fdi_lanes > 2)
+ WARN_ON(I915_READ(SOUTH_CHICKEN1) &
FDI_BC_BIFURCATION_SELECT);
+ else
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ case PIPE_C:
+ cpt_enable_fdi_bc_bifurcation(dev);
+
+ break;
+ default:
+ BUG();
+ }
+}
+
/*
* Enable PCH resources required for PCH ports:
* - PCH PLLs
@@ -3092,6 +3135,9 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
assert_pch_transcoder_disabled(dev_priv, pipe);
+ if (IS_IVYBRIDGE(dev))
+ ivybridge_update_fdi_bc_bifurcation(intel_crtc);
+
/* Write the TU size bits before fdi link training, so that error
* detection works. */
I915_WRITE(FDI_RX_TUSIZE1(pipe),
@@ -5849,48 +5895,6 @@ static bool ironlake_compute_clocks(struct drm_crtc
*crtc,
return true;
}
-static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- uint32_t temp;
-
- temp = I915_READ(SOUTH_CHICKEN1);
- if (temp & FDI_BC_BIFURCATION_SELECT)
- return;
-
- WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
- WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
-
- temp |= FDI_BC_BIFURCATION_SELECT;
- DRM_DEBUG_KMS("enabling fdi C rx\n");
- I915_WRITE(SOUTH_CHICKEN1, temp);
- POSTING_READ(SOUTH_CHICKEN1);
-}
-
-static void ivybridge_update_fdi_
