Re: [Intel-gfx] [PATCH v2 4/5] drm/i915: Change locking for struct_mutex, v2.
Op 02-11-15 om 14:06 schreef Chris Wilson: > On Mon, Nov 02, 2015 at 01:57:59PM +0100, Maarten Lankhorst wrote: >> struct_mutex is being locked for every plane in intel_prepare_plane_fb and >> intel_cleanup_plane_fb. This can be optimized by acquiring struct_mutex first >> before calling the atomic helpers. This way the lock only needs to be >> acquired >> twice in ->atomic_commit(). Once for pinning new framebuffers at the start, >> the second time for unpinning old framebuffer. > A little explanation that you move the locking into the caller would > help clarify the patch. > >> @@ -13453,10 +13461,6 @@ intel_prepare_plane_fb(struct drm_plane *plane, >> @@ -13520,7 +13521,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > Please document that you now expect both of these functions to be called > with struct_mutex held. Also is there any opportunity to reduce the > struct_mutex lock time? > Patch 5/5 reduces locking time. It moves most waiting out of struct_mutex except for the full idle during modesets. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Describe the Rotation property bits.
On mån, 2015-11-02 at 16:50 +0200, Ville Syrjälä wrote: > On Mon, Nov 02, 2015 at 03:24:15PM +0100, Robert Fekete wrote: > > Adds clarification of the rotation property bits. I.e. rotation is > > Counter clockwise and that reflects are applied before any rotation. > > > > Signed-off-by: Robert Fekete> > --- > > include/drm/drm_crtc.h | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index 3f0c6909dda1..45334fa0cb10 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -85,7 +85,11 @@ static inline uint64_t I642U64(int64_t val) > > return (uint64_t)*((uint64_t *)); > > } > > > > -/* rotation property bits */ > > +/* > > + * Rotation property bits. Rotate-(degrees) > > the "rotate-" and "reflect-" things were supposed to be the names of the > property bits. I guess some quotes around them would have been good. > > In any case, I tink here we should refer to the define names. eg > "DRM_ROTATE_ rotates the image by the ..." Thanks for noticing. You are absolutely right, V2 coming up in a sec. > > rotates the image by the specified > > + * amount in degrees in counter clockwise direction. reflect-x and > > reflect-y > > same here with > "DRM_REFLECT_X and DRM_REFLECT_Y ..." Yes! > > + * reflects the image along the specified axis prior to rotation > > + */ > > #define DRM_ROTATE_MASK 0x0f > > #define DRM_ROTATE_0 0 > > #define DRM_ROTATE_90 1 > > -- > > 1.9.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > -- BR /Robert Fekete Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm: Describe the Rotation property bits.
On Mon, Nov 02, 2015 at 04:14:08PM +0100, Robert Fekete wrote: > Adds clarification of the rotation property bits. I.e. rotation is > counter clockwise and that reflects are applied before any rotation. > > v2: Refer to the define names instead of the property values. > > Signed-off-by: Robert FeketeReviewed-by: Ville Syrjälä > --- > include/drm/drm_crtc.h | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 3f0c6909dda1..f47cefa37ef3 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -85,7 +85,11 @@ static inline uint64_t I642U64(int64_t val) > return (uint64_t)*((uint64_t *)); > } > > -/* rotation property bits */ > +/* > + * Rotation property bits. DRM_ROTATE_ rotates the image by the > + * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X > and > + * DRM_REFLECT_Y reflects the image along the specified axis prior to > rotation > + */ > #define DRM_ROTATE_MASK 0x0f > #define DRM_ROTATE_0 0 > #define DRM_ROTATE_901 > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote: > Hey, > > Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > > From: Ville Syrjälä> > > > This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9. > > > > commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully interruptible.") > > breaks GPU reset on gen3/4 machines. Go back to to non-interruptible. > > > I've done some digging and by forcing an unconditional modeset during reset I > was able to trigger it on my system. Hmm, maybe we should add some kind of debug knob to do just that. That way we could test most of the gen3/4 reset path with gen5+. I thought we already had that for load detection, but I may have imagined it considering that load detection seems to have been broken now for a while. > It should be fixed by applying the rest of the interruptible > series so we can mask EIO, followed by replacing > i915_mutex_lock_interruptible with mutex_lock_interruptible so there will be > no waiting for gpu reset. > This is what's causing the deadlock here. :) > > ~Maarten > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 22c0f8a54053..df6dbbc85855 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3207,9 +3207,11 @@ void intel_prepare_reset(struct drm_device *dev) > if (IS_GEN2(dev)) > return; > > +#if 0 > /* reset doesn't touch the display */ > if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) > return; > +#endif > > drm_modeset_lock_all(dev); > /* > @@ -3245,7 +3247,12 @@ void intel_finish_reset(struct drm_device *dev) >* FIXME: Atomic will make this obsolete since we won't schedule >* CS-based flips (which might get lost in gpu resets) any more. >*/ > +#if 0 > intel_update_primary_planes(dev); > +#else > + intel_display_resume(dev); > + drm_modeset_unlock_all(dev); > +#endif > return; > } > > @@ -13174,12 +13181,12 @@ static int intel_atomic_prepare_commit(struct > drm_device *dev, > flush_workqueue(dev_priv->wq); > } > > - ret = i915_mutex_lock_interruptible(dev); > + ret = mutex_lock_interruptible(>struct_mutex); > if (ret) > return ret; > > ret = drm_atomic_helper_prepare_planes(dev, state); > - if (!ret && !async) { > + if (!ret && !async && !i915_reset_in_progress(_priv->gpu_error)) { > u32 reset_counter; > > reset_counter = atomic_read(_priv->gpu_error.reset_counter); -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Wait for object idle without locks in atomic_commit.
On Mon, Nov 02, 2015 at 02:13:48PM +0100, Maarten Lankhorst wrote: > Op 29-10-15 om 01:30 schreef Matt Roper: > > On Wed, Sep 23, 2015 at 01:27:12PM +0200, Maarten Lankhorst wrote: > >> diff --git a/drivers/gpu/drm/i915/intel_overlay.c > >> b/drivers/gpu/drm/i915/intel_overlay.c > >> index 444542696a2c..1b18cc6bdbd6 100644 > >> --- a/drivers/gpu/drm/i915/intel_overlay.c > >> +++ b/drivers/gpu/drm/i915/intel_overlay.c > >> @@ -749,7 +749,11 @@ static int intel_overlay_do_put_image(struct > >> intel_overlay *overlay, > >>if (ret != 0) > >>return ret; > >> > >> - ret = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL, NULL, > >> + ret = i915_gem_object_wait_rendering(new_bo, true); > > Again, I'm not super familiar with GEM internals...can this be a > > behavior change from the previous code? Originally the pin_to_display > > plane function would have passed (obj->base.pending_write_domain == 0) > > as the second parameter here (readonly), but you're unconditionally > > passing true. Can there not be pending writes against this object? > I don't think it would be important in the case of overlays. But maybe I > should > just replace it with a call to i915_gem_object_sync and wait for full object > idle. Technically it removes a call to set-cache, acquiring a GGTT offset and pinning it, and a final set-to-gtt. Quite a major and broken change. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Wait for object idle without locks in atomic_commit.
On Mon, Nov 02, 2015 at 02:53:00PM +0100, Maarten Lankhorst wrote: > Op 02-11-15 om 14:46 schreef Chris Wilson: > > On Mon, Nov 02, 2015 at 02:13:48PM +0100, Maarten Lankhorst wrote: > >> Op 29-10-15 om 01:30 schreef Matt Roper: > >>> On Wed, Sep 23, 2015 at 01:27:12PM +0200, Maarten Lankhorst wrote: > diff --git a/drivers/gpu/drm/i915/intel_overlay.c > b/drivers/gpu/drm/i915/intel_overlay.c > index 444542696a2c..1b18cc6bdbd6 100644 > --- a/drivers/gpu/drm/i915/intel_overlay.c > +++ b/drivers/gpu/drm/i915/intel_overlay.c > @@ -749,7 +749,11 @@ static int intel_overlay_do_put_image(struct > intel_overlay *overlay, > if (ret != 0) > return ret; > > -ret = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL, > NULL, > +ret = i915_gem_object_wait_rendering(new_bo, true); > >>> Again, I'm not super familiar with GEM internals...can this be a > >>> behavior change from the previous code? Originally the pin_to_display > >>> plane function would have passed (obj->base.pending_write_domain == 0) > >>> as the second parameter here (readonly), but you're unconditionally > >>> passing true. Can there not be pending writes against this object? > >> I don't think it would be important in the case of overlays. But maybe I > >> should > >> just replace it with a call to i915_gem_object_sync and wait for full > >> object idle. > > Technically it removes a call to set-cache, acquiring a GGTT offset and > > pinning it, and a final set-to-gtt. > > > > Quite a major and broken change. > > -Chris > > > No? pin_to_display_plane is called immediately below in this patch, it's just > not shown here in the comments because it was about using wait_rendering. Bah. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v3)
On 27 October 2015 at 10:37, Tvrtko Ursulinwrote: > > On 23/10/15 12:35, Daniel Vetter wrote: >> >> On Fri, Oct 23, 2015 at 09:51:06AM +0100, Tvrtko Ursulin wrote: >>> >>> >>> Hi, >>> >>> On 23/10/15 02:34, Vivek Kasireddy wrote: The main goal of this subtest is to trigger the following warning in the function i915_gem_object_get_fence(): if (WARN_ON(!obj->map_and_fenceable)) To trigger this warning, the subtest first creates a Y-tiled object and an associated framebuffer with the Y-fb modifier. Furthermore, to prevent the map_and_fenceable from being set, we make sure that the object does not have a normal VMA by refraining from rendering to the object and by setting the rotation property upfront before calling commit. v2: Do not call paint_squares and just use one output. v3: Convert an if condition to igt_require and move the plane rotation requirement further up before the fb allocation. Cc: Tvrtko Ursulin Signed-off-by: Vivek Kasireddy --- tests/kms_rotation_crc.c | 68 1 file changed, 68 insertions(+) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index cc9847e..b25a949 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -264,6 +264,68 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) igt_require_f(valid_tests, "no valid crtc/connector combinations found\n"); } +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type) +{ + igt_display_t *display = >display; + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + uint32_t format = DRM_FORMAT_XRGB; + int bpp = igt_drm_format_to_bpp(format); + enum igt_commit_style commit = COMMIT_LEGACY; + int fd = data->gfx_fd; + igt_output_t *output = >outputs[0]; + igt_plane_t *plane; + drmModeModeInfo *mode; + unsigned int stride, size, w, h; + uint32_t gem_handle; + int ret; + + igt_require(output != NULL && output->valid == true); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + + if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + for (stride = 512; stride < (w * bpp / 8); stride *= 2) + ; + for (size = 1024*1024; size < stride * h; size *= 2) + ; + + gem_handle = gem_create(fd, size); + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride); + igt_assert(ret == 0); + + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, + >fb.fb_id)); + data->fb.width = w; + data->fb.height = h; + data->fb.gem_handle = gem_handle; + + igt_plane_set_fb(plane, NULL); + igt_display_commit(display); + + igt_plane_set_rotation(plane, data->rotation); + igt_plane_set_fb(plane, >fb); + + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, +DRM_MODE_OBJECT_PLANE, +plane->rotation_property, +plane->rotation); + ret = igt_display_try_commit2(display, commit); + + kmstest_restore_vt_mode(); + igt_remove_fb(fd, >fb); + igt_assert(ret == 0); +} + igt_main { data_t data = {}; @@ -345,6 +407,12 @@ igt_main test_plane_rotation(, IGT_PLANE_PRIMARY); } + igt_subtest_f("primary-rotation-90-Y-tiled") { + igt_require(gen >= 9); + data.rotation = IGT_ROTATION_90; + test_plane_rotation_ytiled_obj(, IGT_PLANE_PRIMARY); + } + igt_fixture { igt_display_fini(); } >>> >>> Reviewed-by: Tvrtko Ursulin >> >> >> Applied, thanks. > > > Hasn't hit the repo yet. Daniel has now pushed this patch (v3), so the further changes that have been made will need to be split into new patches. > > Regards, > > Tvrtko > >
[Intel-gfx] [PATCH] drm: Describe the Rotation property bits.
Adds clarification of the rotation property bits. I.e. rotation is Counter clockwise and that reflects are applied before any rotation. Signed-off-by: Robert Fekete--- include/drm/drm_crtc.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3f0c6909dda1..45334fa0cb10 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -85,7 +85,11 @@ static inline uint64_t I642U64(int64_t val) return (uint64_t)*((uint64_t *)); } -/* rotation property bits */ +/* + * Rotation property bits. Rotate-(degrees) rotates the image by the specified + * amount in degrees in counter clockwise direction. reflect-x and reflect-y + * reflects the image along the specified axis prior to rotation + */ #define DRM_ROTATE_MASK 0x0f #define DRM_ROTATE_0 0 #define DRM_ROTATE_90 1 -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Wait for object idle without locks in atomic_commit.
Op 02-11-15 om 14:46 schreef Chris Wilson: > On Mon, Nov 02, 2015 at 02:13:48PM +0100, Maarten Lankhorst wrote: >> Op 29-10-15 om 01:30 schreef Matt Roper: >>> On Wed, Sep 23, 2015 at 01:27:12PM +0200, Maarten Lankhorst wrote: diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 444542696a2c..1b18cc6bdbd6 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -749,7 +749,11 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay, if (ret != 0) return ret; - ret = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL, NULL, + ret = i915_gem_object_wait_rendering(new_bo, true); >>> Again, I'm not super familiar with GEM internals...can this be a >>> behavior change from the previous code? Originally the pin_to_display >>> plane function would have passed (obj->base.pending_write_domain == 0) >>> as the second parameter here (readonly), but you're unconditionally >>> passing true. Can there not be pending writes against this object? >> I don't think it would be important in the case of overlays. But maybe I >> should >> just replace it with a call to i915_gem_object_sync and wait for full object >> idle. > Technically it removes a call to set-cache, acquiring a GGTT offset and > pinning it, and a final set-to-gtt. > > Quite a major and broken change. > -Chris > No? pin_to_display_plane is called immediately below in this patch, it's just not shown here in the comments because it was about using wait_rendering. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fall back to zero vswing/preemph if the sink doesn't like the last good values
On Mon, Nov 02, 2015 at 01:21:23PM +, Conselvan De Oliveira, Ander wrote: > On Fri, 2015-10-30 at 18:47 +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä> > > > My Lenovo STM STDP3100 miniDP->VGA dongle doesn't seem to like it when > > we try to start link training with non-zero vswing/preemphasis. So when > > the initial link training DPCD write fails, retry it with zero values. > > Does the device NACKs the request? That was my assumption, but actually it just defers it to death. I had to bump the retry count to ~48 to make it work without this patch. That was with drm.debug=0, with drm.debug=0xe it seems to work even with the current retry count of 32. Which also makes me think we should run the bat stuff both with debugs on and off to catch more problems. > > Ander > > > > > Fixes a bunch of errors like so: > > [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link > > training > > > > Cc: Mika Kahola > > Cc: Sivakumar Thulasimani > > Cc: Ander Conselvan de Oliveira > > Fixes: 5fa836a9d859 ("drm/i915: DP link training optimization") > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_dp.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index ba4cbf5..9529a6e 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3750,10 +3750,17 @@ intel_dp_link_training_clock_recovery(struct > > intel_dp > > *intel_dp) > > > > DP |= DP_PORT_EN; > > > > +again: > > /* clock recovery */ > > if (!intel_dp_reset_link_train(intel_dp, , > >DP_TRAINING_PATTERN_1 | > >DP_LINK_SCRAMBLING_DISABLE)) { > > + if (intel_dp->train_set_valid) { > > + DRM_DEBUG_KMS("Sink rejected link training request, > > trying again with zero values\n"); > > + intel_dp->train_set_valid = false; > > + goto again; > > + } > > + > > DRM_ERROR("failed to enable link training\n"); > > return; > > } -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 02:00:47PM +, Gong, Zhipeng wrote: > Attach the perf data for BDW async1 and async5 with or without patch. Hmm, I can see it is the i915_spin_request() consuming the time, but I was hoping to get the callgraph so I could see where the call to i915_wait_request was originating from. Could you regenerate the perf report with -g (or maybe you need a slightly newer perf)? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm: Describe the Rotation property bits.
Adds clarification of the rotation property bits. I.e. rotation is counter clockwise and that reflects are applied before any rotation. v2: Refer to the define names instead of the property values. Signed-off-by: Robert Fekete--- include/drm/drm_crtc.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3f0c6909dda1..f47cefa37ef3 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -85,7 +85,11 @@ static inline uint64_t I642U64(int64_t val) return (uint64_t)*((uint64_t *)); } -/* rotation property bits */ +/* + * Rotation property bits. DRM_ROTATE_ rotates the image by the + * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X and + * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation + */ #define DRM_ROTATE_MASK 0x0f #define DRM_ROTATE_0 0 #define DRM_ROTATE_90 1 -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 02:57:47PM +, Gong, Zhipeng wrote: > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: Monday, November 02, 2015 10:16 PM > > To: Gong, Zhipeng > > Cc: intel-gfx@lists.freedesktop.org; Rogozhkin, Dmitry V > > Subject: Re: [PATCH] RFC drm/i915: Slaughter the thundering > > i915_wait_request herd > > > > On Mon, Nov 02, 2015 at 02:00:47PM +, Gong, Zhipeng wrote: > > > Attach the perf data for BDW async1 and async5 with or without patch. > > > > Hmm, I can see it is the i915_spin_request() consuming the time, but I was > > hoping to get the callgraph so I could see where the call to > > i915_wait_request > > was originating from. Could you regenerate the perf report with -g (or maybe > > you need a slightly newer perf)? > > -Chris > > > Chris, I dump the it to txt, but it is hard to read since the call chain is > too long. > If you need the data in other format, please let me know. Just get a wider monitor :) Ok, that does confirm that it is the frequent inter-ring sync keeping the waitqueue short. If you try, diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 21017daecb90..008b43be791f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -70,6 +70,8 @@ static void intel_engine_irq_wakeup(struct work_struct *work) container_of(work, struct intel_engine_cs, irq_work); const bool noirq = !__irq_enable(engine); DEFINE_WAIT(wait); +#define KEEPALIVE 10 + int keepalive = KEEPALIVE; for (;;) { struct timer_list timer; @@ -115,8 +117,11 @@ static void intel_engine_irq_wakeup(struct work_struct *work) } engine->irq_first = request; spin_unlock(>irq_lock); - if (request == NULL) - break; + if (request == NULL) { + if (--keepalive == 0) + break; + } else + keepalive = KEEPALIVE; /* Make sure the hangcheck timer is armed in case the GPU * hangs and the request never completes. @@ -124,7 +129,7 @@ static void intel_engine_irq_wakeup(struct work_struct *work) i915_queue_hangcheck(engine->i915); timer.function = NULL; - if (noirq || missed_irq(engine)) { + if (request == NULL || noirq || missed_irq(engine)) { setup_timer_on_stack(, (void (*)(unsigned long))wake_up_process, (unsigned long)current); @@ -165,6 +170,8 @@ void intel_engine_add_wakeup(struct drm_i915_gem_request *request) if (RB_EMPTY_ROOT(>irq_requests)) schedule_work(>irq_work); + else + wake_up_all(>irq_queue); init_waitqueue_head(>wait); That should keep the worker alive for a further 10 jiffies, hopefully long enough for the next wait to occur. The cost is that it keeps the interrupt asserted (and to avoid that requires a little rearrangment and probably a dedicated kthread for each ring). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/5] drm/i915: Change locking for struct_mutex, v2.
struct_mutex is being locked for every plane in intel_prepare_plane_fb and intel_cleanup_plane_fb. This can be optimized by acquiring struct_mutex first before calling the atomic helpers. This way the lock only needs to be acquired twice in ->atomic_commit(). Once for pinning new framebuffers at the start, the second time for unpinning old framebuffer. Changes since v1: - Use mutex_lock_interruptible instead of i915 variant, to prevent a deadlock when atomic_commit is called from the reset code. Signed-off-by: Maarten LankhorstReviewed-by: Matt Roper --- Does this look better? drivers/gpu/drm/i915/intel_display.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 36e7e29ea266..13aaae38f7f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13165,8 +13165,13 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, return ret; } + ret = mutex_lock_interruptible(>struct_mutex); + if (ret) + return ret; + ret = drm_atomic_helper_prepare_planes(dev, state); + mutex_unlock(>struct_mutex); return ret; } @@ -13268,7 +13273,10 @@ static int intel_atomic_commit(struct drm_device *dev, /* FIXME: add subpixel order */ drm_atomic_helper_wait_for_vblanks(dev, state); + + mutex_lock(>struct_mutex); drm_atomic_helper_cleanup_planes(dev, state); + mutex_unlock(>struct_mutex); if (any_ms) intel_modeset_check_state(dev, state); @@ -13453,10 +13461,6 @@ intel_prepare_plane_fb(struct drm_plane *plane, if (!obj && !old_obj) return 0; - ret = i915_mutex_lock_interruptible(dev); - if (ret) - return ret; - if (old_obj) { struct drm_crtc_state *crtc_state = drm_atomic_get_existing_crtc_state(new_state->state, plane->state->crtc); @@ -13477,7 +13481,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, /* Swallow -EIO errors to allow updates during hw lockup. */ if (ret && ret != -EIO) - goto out; + return ret; } if (!obj) { @@ -13495,9 +13499,6 @@ intel_prepare_plane_fb(struct drm_plane *plane, if (ret == 0) i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit); -out: - mutex_unlock(>struct_mutex); - return ret; } @@ -13520,7 +13521,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, if (!obj && !old_obj) return; - mutex_lock(>struct_mutex); if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || !INTEL_INFO(dev)->cursor_needs_physical)) intel_unpin_fb_obj(old_state->fb, old_state); @@ -13529,7 +13529,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) || (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit))) i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit); - mutex_unlock(>struct_mutex); } int -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On Mon, 02 Nov 2015, Shobhit Kumarwrote: > SWF18 is set if the display has been intialized by the pre-os. It > also gives what configuration is enabled on which pipe. The DPLL and > CDCLK verification checks can fail as the pre-os does initialize the > DPLL for Audio codec initialization. So fisrt check if SWF18 is set and > then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? BR, Jani. > > Cc: Ville Syrjälä > Signed-off-by: Shobhit Kumar > --- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 8 > 2 files changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 9ee9481..bd476ff 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { > #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) > #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) > > +/* VBIOS flag for display initialized status */ > +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) > + > /* Pipe B */ > #define _PIPEBDSL(dev_priv->info.display_mmio_offset + 0x71000) > #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 103cacb..0ecb35c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private > *dev_priv) > uint32_t cdctl = I915_READ(CDCLK_CTL); > int freq = dev_priv->skl_boot_cdclk; > > + /* > + * check if the pre-os intialized the display > + * There is SWF18 scratchpad register defined which is set by the > + * pre-os which can be used by the OS drivers to check the status > + */ > + if ((I915_READ(GEN6_SWF18) & 0x00) == 0) > + goto sanitize; > + > /* Is PLL enabled and locked ? */ > if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) > goto sanitize; -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect
Hi Shubhangi, [auto build test WARNING on drm-intel/for-linux-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base] url: https://github.com/0day-ci/linux/commits/Shubhangi-Shrivastava/Fixing-sink-count-related-detection-over/20151102-205435 config: i386-randconfig-x003-11010709 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/gpu/drm/i915/intel_dp.c: In function 'intel_dp_detect': >> drivers/gpu/drm/i915/intel_dp.c:4883:22: warning: passing argument 1 of >> 'intel_dp_long_pulse' from incompatible pointer type >> [-Wincompatible-pointer-types] intel_dp_long_pulse(intel_dp->attached_connector); ^ drivers/gpu/drm/i915/intel_dp.c:4789:1: note: expected 'struct drm_connector *' but argument is of type 'struct intel_connector *' intel_dp_long_pulse(struct drm_connector *connector) ^ vim +/intel_dp_long_pulse +4883 drivers/gpu/drm/i915/intel_dp.c 4867 { 4868 struct intel_dp *intel_dp = intel_attached_dp(connector); 4869 struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); 4870 struct intel_encoder *intel_encoder = _dig_port->base; 4871 struct intel_connector *intel_connector = to_intel_connector(connector); 4872 4873 DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", 4874connector->base.id, connector->name); 4875 4876 if (intel_dp->is_mst) { 4877 /* MST devices are disconnected from a monitor POV */ 4878 if (intel_encoder->type != INTEL_OUTPUT_EDP) 4879 intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; 4880 return connector_status_disconnected; 4881 } 4882 > 4883 intel_dp_long_pulse(intel_dp->attached_connector); 4884 4885 if (intel_connector->detect_edid) 4886 return connector_status_connected; 4887 else 4888 return connector_status_disconnected; 4889 } 4890 4891 static void --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915: Change locking for struct_mutex, v2.
On Mon, Nov 02, 2015 at 01:57:59PM +0100, Maarten Lankhorst wrote: > struct_mutex is being locked for every plane in intel_prepare_plane_fb and > intel_cleanup_plane_fb. This can be optimized by acquiring struct_mutex first > before calling the atomic helpers. This way the lock only needs to be acquired > twice in ->atomic_commit(). Once for pinning new framebuffers at the start, > the second time for unpinning old framebuffer. A little explanation that you move the locking into the caller would help clarify the patch. > @@ -13453,10 +13461,6 @@ intel_prepare_plane_fb(struct drm_plane *plane, > @@ -13520,7 +13521,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, Please document that you now expect both of these functions to be called with struct_mutex held. Also is there any opportunity to reduce the struct_mutex lock time? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Splitting intel_dp_check_link_status
Hi Shubhangi, [auto build test WARNING on drm-intel/for-linux-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base] url: https://github.com/0day-ci/linux/commits/Shubhangi-Shrivastava/Fixing-sink-count-related-detection-over/20151102-205435 config: i386-randconfig-x003-11010709 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): drivers/gpu/drm/i915/intel_dp.c: In function 'intel_dp_short_pulse': >> drivers/gpu/drm/i915/intel_dp.c:4461:24: warning: unused variable >> 'intel_encoder' [-Wunused-variable] struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; ^ drivers/gpu/drm/i915/intel_dp.c: In function 'intel_dp_detect': drivers/gpu/drm/i915/intel_dp.c:4898:23: warning: passing argument 1 of 'intel_dp_long_pulse' from incompatible pointer type [-Wincompatible-pointer-types] intel_dp_long_pulse(intel_dp->attached_connector); ^ drivers/gpu/drm/i915/intel_dp.c:4803:1: note: expected 'struct drm_connector *' but argument is of type 'struct intel_connector *' intel_dp_long_pulse(struct drm_connector *connector) ^ drivers/gpu/drm/i915/intel_dp.c: In function 'intel_dp_hpd_pulse': drivers/gpu/drm/i915/intel_dp.c:5228:23: warning: passing argument 1 of 'intel_dp_long_pulse' from incompatible pointer type [-Wincompatible-pointer-types] intel_dp_long_pulse(intel_dp->attached_connector); ^ drivers/gpu/drm/i915/intel_dp.c:4803:1: note: expected 'struct drm_connector *' but argument is of type 'struct intel_connector *' intel_dp_long_pulse(struct drm_connector *connector) ^ vim +/intel_encoder +4461 drivers/gpu/drm/i915/intel_dp.c b2008584e Shubhangi Shrivastava 2015-11-02 4445 intel_dp_stop_link_train(intel_dp); b2008584e Shubhangi Shrivastava 2015-11-02 4446} b2008584e Shubhangi Shrivastava 2015-11-02 4447 } b2008584e Shubhangi Shrivastava 2015-11-02 4448 a4fc5ed69 Keith Packard 2009-04-07 4449 /* a4fc5ed69 Keith Packard 2009-04-07 4450 * According to DP spec a4fc5ed69 Keith Packard 2009-04-07 4451 * 5.1.2: a4fc5ed69 Keith Packard 2009-04-07 4452 * 1. Read DPCD a4fc5ed69 Keith Packard 2009-04-07 4453 * 2. Configure link according to Receiver Capabilities a4fc5ed69 Keith Packard 2009-04-07 4454 * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 a4fc5ed69 Keith Packard 2009-04-07 4455 * 4. Check link status on receipt of hot-plug interrupt a4fc5ed69 Keith Packard 2009-04-07 4456 */ a51462004 Damien Lespiau2015-02-10 4457 static void b2008584e Shubhangi Shrivastava 2015-11-02 4458 intel_dp_short_pulse(struct intel_dp *intel_dp) a4fc5ed69 Keith Packard 2009-04-07 4459 { 5b215bcff Dave Airlie 2014-08-05 4460struct drm_device *dev = intel_dp_to_dev(intel_dp); da63a9f2e Paulo Zanoni 2012-10-26 @4461struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; a60f0e38d Jesse Barnes 2011-10-20 4462u8 sink_irq_vector; 93f62dad5 Keith Packard 2011-11-01 4463u8 link_status[DP_LINK_STATUS_SIZE]; a60f0e38d Jesse Barnes 2011-10-20 4464 92fd8fd13 Keith Packard 2011-07-25 4465/* Try to read receiver status if the link appears to be up */ 93f62dad5 Keith Packard 2011-11-01 4466if (!intel_dp_get_link_status(intel_dp, link_status)) { a4fc5ed69 Keith Packard 2009-04-07 4467return; a4fc5ed69 Keith Packard 2009-04-07 4468} a4fc5ed69 Keith Packard 2009-04-07 4469 :: The code at line 4461 was first introduced by commit :: da63a9f2e4a1595f4890e38a0511d74bea1a51f0 drm/i915: create intel_digital_port and use it :: TO: Paulo Zanoni <paulo.r.zan...@intel.com> :: CC: Daniel Vetter <daniel.vet...@ffwll.ch> --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Wait for object idle without locks in atomic_commit.
Op 29-10-15 om 01:30 schreef Matt Roper: > On Wed, Sep 23, 2015 at 01:27:12PM +0200, Maarten Lankhorst wrote: >> Make pinning and waiting a separate step, and wait for object idle >> without struct_mutex held. >> >> Signed-off-by: Maarten Lankhorst>> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 - >> drivers/gpu/drm/i915/i915_gem.c | 6 --- >> drivers/gpu/drm/i915/intel_atomic_plane.c | 2 + >> drivers/gpu/drm/i915/intel_display.c | 86 >> ++- >> drivers/gpu/drm/i915/intel_drv.h | 7 +-- >> drivers/gpu/drm/i915/intel_fbdev.c| 2 +- >> drivers/gpu/drm/i915/intel_overlay.c | 6 ++- >> 7 files changed, 84 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 3bf8a9b771d0..ec72fd457499 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2982,8 +2982,6 @@ i915_gem_object_set_to_cpu_domain(struct >> drm_i915_gem_object *obj, bool write); >> int __must_check >> i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, >> u32 alignment, >> - struct intel_engine_cs *pipelined, >> - struct drm_i915_gem_request >> **pipelined_request, >> const struct i915_ggtt_view *view); >> void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object >> *obj, >>const struct i915_ggtt_view >> *view); >> diff --git a/drivers/gpu/drm/i915/i915_gem.c >> b/drivers/gpu/drm/i915/i915_gem.c >> index 46f0e83ee6ee..ab02182c47a5 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -3782,17 +3782,11 @@ unlock: >> int >> i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, >> u32 alignment, >> - struct intel_engine_cs *pipelined, >> - struct drm_i915_gem_request >> **pipelined_request, >> const struct i915_ggtt_view *view) >> { >> u32 old_read_domains, old_write_domain; >> int ret; >> >> -ret = i915_gem_object_sync(obj, pipelined, pipelined_request); >> -if (ret) >> -return ret; >> - >> /* Mark the pin_display early so that we account for the >> * display coherency whilst setting up the cache domains. >> */ >> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c >> b/drivers/gpu/drm/i915/intel_atomic_plane.c >> index a11980696595..c6bb0fc1edfb 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c >> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c >> @@ -84,6 +84,7 @@ intel_plane_duplicate_state(struct drm_plane *plane) >> state = _state->base; >> >> __drm_atomic_helper_plane_duplicate_state(plane, state); >> +intel_state->wait_req = NULL; >> >> return state; >> } >> @@ -100,6 +101,7 @@ void >> intel_plane_destroy_state(struct drm_plane *plane, >>struct drm_plane_state *state) >> { >> +WARN_ON(state && to_intel_plane_state(state)->wait_req); >> drm_atomic_helper_plane_destroy_state(plane, state); >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 2f046134cc9a..d817c44ee428 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -2291,9 +2291,7 @@ static unsigned int intel_linear_alignment(struct >> drm_i915_private *dev_priv) >> int >> intel_pin_and_fence_fb_obj(struct drm_plane *plane, >> struct drm_framebuffer *fb, >> - const struct drm_plane_state *plane_state, >> - struct intel_engine_cs *pipelined, >> - struct drm_i915_gem_request **pipelined_request) >> + const struct drm_plane_state *plane_state) >> { >> struct drm_device *dev = fb->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> @@ -2349,8 +2347,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, >> */ >> intel_runtime_pm_get(dev_priv); >> >> -ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined, >> - pipelined_request, ); >> +ret = i915_gem_object_pin_to_display_plane(obj, alignment, >> + ); > The pin to display plane (and hence the wait) happens inside > intel_runtime_pm_get/put() in the current code. When you pull the wait > out to the various callsites, it isn't holding runtime pm anymore (at > least not in a way that's obvious to me). Can this be a problem? > Neither runtime PM nor GEM internals are something I'm terribly familiar > with, so you might want
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On 11/02/2015 06:40 PM, Jani Nikula wrote: On Mon, 02 Nov 2015, Shobhit Kumarwrote: SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? As per my discussion with gop team, this has been enabled in main stream for quite sometime including VLV, CHT, BDW, SKL+ and is common for GOP/VBIOS across chrome/windows/android. So yes I think we can universally trust as of now. Regards Shobhit BR, Jani. Cc: Ville Syrjälä Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF(dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; + /* +* check if the pre-os intialized the display +* There is SWF18 scratchpad register defined which is set by the +* pre-os which can be used by the OS drivers to check the status +*/ + if ((I915_READ(GEN6_SWF18) & 0x00) == 0) + goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fall back to zero vswing/preemph if the sink doesn't like the last good values
On Fri, 2015-10-30 at 18:47 +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > My Lenovo STM STDP3100 miniDP->VGA dongle doesn't seem to like it when > we try to start link training with non-zero vswing/preemphasis. So when > the initial link training DPCD write fails, retry it with zero values. Does the device NACKs the request? Ander > > Fixes a bunch of errors like so: > [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training > > Cc: Mika Kahola > Cc: Sivakumar Thulasimani > Cc: Ander Conselvan de Oliveira > Fixes: 5fa836a9d859 ("drm/i915: DP link training optimization") > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dp.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ba4cbf5..9529a6e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3750,10 +3750,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp > *intel_dp) > > DP |= DP_PORT_EN; > > +again: > /* clock recovery */ > if (!intel_dp_reset_link_train(intel_dp, , > DP_TRAINING_PATTERN_1 | > DP_LINK_SCRAMBLING_DISABLE)) { > + if (intel_dp->train_set_valid) { > + DRM_DEBUG_KMS("Sink rejected link training request, > trying again with zero values\n"); > + intel_dp->train_set_valid = false; > + goto again; > + } > + > DRM_ERROR("failed to enable link training\n"); > return; > } - Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On 11/2/2015 11:17 PM, Kumar, Shobhit wrote: On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote: On 11/2/2015 6:49 PM, Kumar, Shobhit wrote: On 11/02/2015 06:40 PM, Jani Nikula wrote: On Mon, 02 Nov 2015, Shobhit Kumarwrote: SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? As per my discussion with gop team, this has been enabled in main stream for quite sometime including VLV, CHT, BDW, SKL+ and is common for GOP/VBIOS across chrome/windows/android. So yes I think we can universally trust as of now. This has been added since IVB timeframe and should be part of VBT spec. but i just encountered an issue in Android Charging OS where there is no modeset and is using the displays enabled by GOP/VBIOS. This patch might break such expectations. Why would this break anything in Android charging UI. Basically this patch only says do cdclock sanitization if display is not enabled by pre-os. In this use case it is already enabled by GOP/VBIOS and the driver any way expects it to be programmed by pre-os in general. So in this scenario, the sanitization logic will not do anything at all because SWF18 will be set and CDCLOCK and DPLL will be properly enabled already and just return false. Fast-modeset should not be broken by this at all. my bad :( should review carefully when sitting late night. Regards Shobhit BR, Jani. Cc: Ville Syrjälä Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; +/* + * check if the pre-os intialized the display + * There is SWF18 scratchpad register defined which is set by the + * pre-os which can be used by the OS drivers to check the status + */ +if ((I915_READ(GEN6_SWF18) & 0x00) == 0) +goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; can you share bit more details on when GOP/VBIOS sets DPLL but does not enable display ? (atleast that is what i understood from the commit message) regards, Sivakumar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote: On 11/2/2015 6:49 PM, Kumar, Shobhit wrote: On 11/02/2015 06:40 PM, Jani Nikula wrote: On Mon, 02 Nov 2015, Shobhit Kumarwrote: SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? As per my discussion with gop team, this has been enabled in main stream for quite sometime including VLV, CHT, BDW, SKL+ and is common for GOP/VBIOS across chrome/windows/android. So yes I think we can universally trust as of now. This has been added since IVB timeframe and should be part of VBT spec. but i just encountered an issue in Android Charging OS where there is no modeset and is using the displays enabled by GOP/VBIOS. This patch might break such expectations. Why would this break anything in Android charging UI. Basically this patch only says do cdclock sanitization if display is not enabled by pre-os. In this use case it is already enabled by GOP/VBIOS and the driver any way expects it to be programmed by pre-os in general. So in this scenario, the sanitization logic will not do anything at all because SWF18 will be set and CDCLOCK and DPLL will be properly enabled already and just return false. Fast-modeset should not be broken by this at all. Regards Shobhit BR, Jani. Cc: Ville Syrjälä Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i)(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i)(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; +/* + * check if the pre-os intialized the display + * There is SWF18 scratchpad register defined which is set by the + * pre-os which can be used by the OS drivers to check the status + */ +if ((I915_READ(GEN6_SWF18) & 0x00) == 0) +goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 01/14] drm/i915: Use passed plane state for sprite planes, v3.
Don't use plane->state directly, use the pointer from commit_plane. Changes since v1: - Fix uses of plane->state->rotation and color key to use the passed state too. - Only pass crtc_state and plane_state to update_plane. Changes since v2: - Rebased. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_drv.h| 10 +-- drivers/gpu/drm/i915/intel_sprite.c | 120 +++- 2 files changed, 67 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d1a6071afabe..16d4627364c5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -647,16 +647,12 @@ struct intel_plane { /* * NOTE: Do not place new plane state fields here (e.g., when adding * new plane properties). New runtime state should now be placed in -* the intel_plane_state structure and accessed via drm_plane->state. +* the intel_plane_state structure and accessed via plane_state. */ void (*update_plane)(struct drm_plane *plane, -struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int crtc_x, int crtc_y, -unsigned int crtc_w, unsigned int crtc_h, -uint32_t x, uint32_t y, -uint32_t src_w, uint32_t src_h); +struct intel_crtc_state *crtc_state, +struct intel_plane_state *plane_state); void (*disable_plane)(struct drm_plane *plane, struct drm_crtc *crtc); int (*check_plane)(struct drm_plane *plane, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 4276c135b9f2..6d3047f9c80f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -178,28 +178,32 @@ void intel_pipe_update_end(struct intel_crtc *crtc) } static void -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, -struct drm_framebuffer *fb, -int crtc_x, int crtc_y, -unsigned int crtc_w, unsigned int crtc_h, -uint32_t x, uint32_t y, -uint32_t src_w, uint32_t src_h) +skl_update_plane(struct drm_plane *drm_plane, +struct intel_crtc_state *crtc_state, +struct intel_plane_state *plane_state) { struct drm_device *dev = drm_plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(drm_plane); + struct drm_framebuffer *fb = plane_state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); const int pipe = intel_plane->pipe; const int plane = intel_plane->plane + 1; u32 plane_ctl, stride_div, stride; - const struct drm_intel_sprite_colorkey *key = - _intel_plane_state(drm_plane->state)->ckey; + const struct drm_intel_sprite_colorkey *key = _state->ckey; unsigned long surf_addr; u32 tile_height, plane_offset, plane_size; unsigned int rotation; int x_offset, y_offset; - struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config; - int scaler_id; + + int crtc_x = plane_state->dst.x1, crtc_y = plane_state->dst.y1; + uint32_t crtc_w = drm_rect_width(_state->dst); + uint32_t crtc_h = drm_rect_height(_state->dst); + uint32_t x = plane_state->src.x1 >> 16, y = plane_state->src.y1 >> 16; + uint32_t src_w = drm_rect_width(_state->src) >> 16; + uint32_t src_h = drm_rect_height(_state->src) >> 16; + const struct intel_scaler *scaler = + _state->scaler_state.scalers[plane_state->scaler_id]; plane_ctl = PLANE_CTL_ENABLE | PLANE_CTL_PIPE_GAMMA_ENABLE | @@ -208,14 +212,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, plane_ctl |= skl_plane_ctl_format(fb->pixel_format); plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]); - rotation = drm_plane->state->rotation; + rotation = plane_state->base.rotation; plane_ctl |= skl_plane_ctl_rotation(rotation); stride_div = intel_fb_stride_alignment(dev, fb->modifier[0], fb->pixel_format); - scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id; - /* Sizes are 0 based */ src_w--; src_h--; @@ -256,13 +258,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, I915_WRITE(PLANE_SIZE(pipe, plane), plane_size); /* program plane scaler */ - if (scaler_id >= 0) { + if (plane_state->scaler_id >= 0) { uint32_t ps_ctrl = 0; + int scaler_id = plane_state->scaler_id;
[Intel-gfx] [PATCH v2 02/14] drm/i915: Extend DSL readout fix to BDW and SKL.
Those platforms have the same bug as haswell, and the same fix applies to them. Signed-off-by: Maarten LankhorstBugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91579 --- drivers/gpu/drm/i915/i915_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6e0a5683bbdc..825114af9c56 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -747,7 +747,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) * problem. We may need to extend this to include other platforms, * but so far testing only shows the problem on HSW. */ - if (IS_HASWELL(dev) && !position) { + if (HAS_DDI(dev) && !position) { int i, temp; for (i = 0; i < 100; i++) { -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 14/14] drm/i915/skl: Do not allow scaling when crtc is disabled.
This fixes a warning when the crtc is turned off. In that case fb will be NULL, and crtc_clock will be 0. Because the crtc is no longer active this is not a bug, and shouldn't trigger the WARN_ON. Also remove handling a null crtc_state, with all transitional helpers gone this can no longer happen. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 304e1028c9a4..7e2caeef9a11 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13671,7 +13671,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state struct drm_i915_private *dev_priv; int crtc_clock, cdclk; - if (!intel_crtc || !crtc_state) + if (!intel_crtc || !crtc_state->base.active) return DRM_PLANE_HELPER_NO_SCALING; dev = intel_crtc->base.dev; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 08/14] drm/i915: Remove intel_crtc->atomic.disable_ips.
This is already handled in pre_disable_primary, disabling it twice is useless. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_display.c | 16 +--- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2708899d9767..58074f4adca2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4777,9 +4777,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) if (atomic->disable_fbc) intel_fbc_disable_crtc(crtc); - if (crtc->atomic.disable_ips) - hsw_disable_ips(crtc); - if (atomic->pre_disable_primary) intel_pre_disable_primary(>base); @@ -11683,19 +11680,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, intel_crtc->atomic.pre_disable_primary = turn_off; intel_crtc->atomic.post_enable_primary = turn_on; - if (turn_off) { - /* -* FIXME: Actually if we will still have any other -* plane enabled on the pipe we could let IPS enabled -* still, but for now lets consider that when we make -* primary invisible by setting DSPCNTR to 0 on -* update_primary_plane function IPS needs to be -* disable. -*/ - intel_crtc->atomic.disable_ips = true; - + if (turn_off) intel_crtc->atomic.disable_fbc = true; - } /* * FBC does not work on some platforms for rotated diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6f99c7398af3..ce5f10fc92aa 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -537,7 +537,6 @@ struct intel_mmio_flip { struct intel_crtc_atomic_commit { /* Sleepable operations to perform before commit */ bool disable_fbc; - bool disable_ips; bool pre_disable_primary; /* Sleepable operations to perform after commit */ -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 11/14] drm/i915: Remove some post-commit members from intel_crtc->atomic.
fb_bits is useful to have in the crtc_state for cs flips later on, so keep it alive there. The other stuff can be calculated in post_plane_update, and aren't useful elsewhere. Currently there's a loop in post_plane_update, this should disappear with the changes to atomic wm's. At that point only updates to the primary plane are important. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_display.c | 29 - drivers/gpu/drm/i915/intel_drv.h | 3 +-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 2ba0cd698f9b..d890f5f63240 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->visible_changed = false; crtc_state->wm_changed = false; crtc_state->fb_changed = false; + crtc_state->fb_bits = 0; return _state->base; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 77fe1d75404b..c42e87c62133 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4742,15 +4742,20 @@ intel_pre_disable_primary(struct drm_crtc *crtc) hsw_disable_ips(intel_crtc); } -static void intel_post_plane_update(struct intel_crtc *crtc) +static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) { + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); + struct drm_atomic_state *old_state = old_crtc_state->base.state; struct intel_crtc_atomic_commit *atomic = >atomic; struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->base.state); struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_plane *primary = crtc->base.primary; + struct drm_plane_state *old_pri_state = + drm_atomic_get_existing_plane_state(old_state, primary); - intel_frontbuffer_flip(dev, atomic->fb_bits); + intel_frontbuffer_flip(dev, pipe_config->fb_bits); crtc->wm.cxsr_allowed = true; @@ -4760,8 +4765,17 @@ static void intel_post_plane_update(struct intel_crtc *crtc) if (atomic->update_fbc) intel_fbc_update(dev_priv); - if (atomic->post_enable_primary) - intel_post_enable_primary(>base); + if (old_pri_state) { + struct intel_plane_state *primary_state = + to_intel_plane_state(primary->state); + struct intel_plane_state *old_primary_state = + to_intel_plane_state(old_pri_state); + + if (primary_state->visible && + (needs_modeset(_config->base) || +!old_primary_state->visible)) + intel_post_enable_primary(>base); + } memset(atomic, 0, sizeof(*atomic)); } @@ -11685,13 +11699,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, } if (visible || was_visible) - intel_crtc->atomic.fb_bits |= - to_intel_plane(plane)->frontbuffer_bit; + pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit; switch (plane->type) { case DRM_PLANE_TYPE_PRIMARY: - intel_crtc->atomic.post_enable_primary = turn_on; - if (turn_off) intel_crtc->atomic.disable_fbc = true; @@ -13389,7 +13400,7 @@ static int intel_atomic_commit(struct drm_device *dev, intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask); for_each_crtc_in_state(state, crtc, crtc_state, i) - intel_post_plane_update(to_intel_crtc(crtc)); + intel_post_plane_update(to_intel_crtc_state(crtc_state)); mutex_lock(>struct_mutex); drm_atomic_helper_cleanup_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dc024c27ca1e..aceeeccec09b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -372,6 +372,7 @@ struct intel_crtc_state { #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */ unsigned long quirks; + unsigned fb_bits; /* framebuffers to flip */ bool update_pipe; /* can a fast modeset be performed? */ bool visible_changed; /* plane visibility changed */ bool wm_changed; /* wm changed */ @@ -539,9 +540,7 @@ struct intel_crtc_atomic_commit { bool disable_fbc; /* Sleepable operations to perform after commit */ - unsigned fb_bits; bool update_fbc; - bool post_enable_primary; }; struct intel_crtc { -- 2.1.0
[Intel-gfx] [PATCH v2 00/14] Kill off intel_crtc->atomic, rework.
After fixing up some earlier patches it turns out I had to rework the rest and improved the individual patches. There were some changes in upstream, which meant I had to redo most of them. The order of patches is slightly changed, the update watermark comes before killing off wait_vblank because it has a dependency on it. Maarten Lankhorst (14): drm/i915: Use passed plane state for sprite planes, v3. drm/i915: Extend DSL readout fix to BDW and SKL. drm/i915: Do not acquire crtc state to check clock during modeset, v2. drm/i915: Handle cdclk limits on broadwell. drm/i915/bxt: Use the bypass frequency if there are no active pipes. drm/i915: Update watermark related members in the crtc_state, v2. drm/i915: Kill off intel_crtc->atomic.wait_vblank. drm/i915: Remove intel_crtc->atomic.disable_ips. drm/i915: Remove atomic.pre_disable_primary. drm/i915: Remove update_sprite_watermarks. drm/i915: Remove some post-commit members from intel_crtc->atomic. drm/i915: Nuke fbc members from intel_crtc->atomic. drm/i915/skl: Update watermarks before the crtc is disabled. drm/i915/skl: Do not allow scaling when crtc is disabled. drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_atomic.c | 6 +- drivers/gpu/drm/i915/intel_display.c | 459 --- drivers/gpu/drm/i915/intel_drv.h | 47 ++-- drivers/gpu/drm/i915/intel_sprite.c | 120 - 6 files changed, 358 insertions(+), 281 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 03/14] drm/i915: Do not acquire crtc state to check clock during modeset, v2.
Parallel modesets are still not allowed, but this will allow updating a different crtc during a modeset if the clock is not changed. Additionally when all pipes are DPMS off the cdclk will be lowered to the minimum allowed. Changes since v1: - Add dev_priv->active_crtcs for tracking which crtcs are active. - Rename min_cdclk to min_pixclk and move to dev_priv. - Add a active_crtcs mask which is updated atomically. - Add intel_atomic_state->modeset which is set on modesets. - Commit new pixclk/active_crtcs right after state swap. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/intel_atomic.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 160 +-- drivers/gpu/drm/i915/intel_drv.h | 7 +- 4 files changed, 125 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 20cd6d8bb083..4a1afcfe904e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1842,8 +1842,13 @@ struct drm_i915_private { struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; #endif + /* dpll and cdclk state is protected by connection_mutex */ int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; + + unsigned int active_crtcs; + unsigned int min_pixclk[I915_MAX_PIPES]; + int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; struct i915_workarounds workarounds; diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 643f342de33b..c4eadbc928b7 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -306,5 +306,5 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) { struct intel_atomic_state *state = to_intel_atomic_state(s); drm_atomic_state_default_clear(>base); - state->dpll_set = false; + state->dpll_set = state->modeset = false; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7b3cfb64e7f6..68f1e0f9d50d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5229,6 +5229,7 @@ static void modeset_put_power_domains(struct drm_i915_private *dev_priv, static void modeset_update_crtc_power_domains(struct drm_atomic_state *state) { + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); struct drm_device *dev = state->dev; struct drm_i915_private *dev_priv = dev->dev_private; unsigned long put_domains[I915_MAX_PIPES] = {}; @@ -5237,13 +5238,18 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state) int i; for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (needs_modeset(crtc->state)) - put_domains[to_intel_crtc(crtc)->pipe] = - modeset_get_crtc_power_domains(crtc); + struct intel_crtc *intel_crtc = + to_intel_crtc(crtc); + enum pipe pipe = intel_crtc->pipe; + + if (!needs_modeset(crtc->state)) + continue; + + put_domains[pipe] = modeset_get_crtc_power_domains(crtc); } if (dev_priv->display.modeset_commit_cdclk) { - unsigned int cdclk = to_intel_atomic_state(state)->cdclk; + unsigned int cdclk = intel_state->cdclk; if (cdclk != dev_priv->cdclk_freq && !WARN_ON(!state->allow_modeset)) @@ -5930,23 +5936,32 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, static int intel_mode_max_pixclk(struct drm_device *dev, struct drm_atomic_state *state) { - struct intel_crtc *intel_crtc; - struct intel_crtc_state *crtc_state; - int max_pixclk = 0; + struct intel_atomic_state *intel_state = to_intel_atomic_state(state); + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + unsigned max_pixel_rate = 0, i; + enum pipe pipe; - for_each_intel_crtc(dev, intel_crtc) { - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); - if (IS_ERR(crtc_state)) - return PTR_ERR(crtc_state); + memcpy(intel_state->min_pixclk, dev_priv->min_pixclk, + sizeof(intel_state->min_pixclk)); - if (!crtc_state->base.enable) - continue; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + int pixclk = 0; + + if (crtc_state->enable) + pixclk = crtc_state->adjusted_mode.crtc_clock; - max_pixclk = max(max_pixclk, -crtc_state->base.adjusted_mode.crtc_clock); +
[Intel-gfx] [PATCH v2 07/14] drm/i915: Kill off intel_crtc->atomic.wait_vblank.
By handling this after the atomic helper waits for vblanks there will be one less wait for vblank in the atomic path. Also get rid of the double wait_for_vblank on broadwell, looks like it's a bug doing the vblank wait twice. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_display.c | 95 +++- drivers/gpu/drm/i915/intel_drv.h | 2 +- 3 files changed, 64 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 3e390db9d22b..2ba0cd698f9b 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -97,6 +97,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->disable_lp_wm = false; crtc_state->visible_changed = false; crtc_state->wm_changed = false; + crtc_state->fb_changed = false; return _state->base; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 356e3a9e1741..2708899d9767 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4669,14 +4669,6 @@ intel_post_enable_primary(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; /* -* BDW signals flip done immediately if the plane -* is disabled, even if the plane enable is already -* armed to occur at the next vblank :( -*/ - if (IS_BROADWELL(dev)) - intel_wait_for_vblank(dev, pipe); - - /* * FIXME IPS should be fine as long as one plane is * enabled, but in practice it seems to have problems * when going from primary only to sprite only and vice @@ -4758,9 +4750,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - if (atomic->wait_vblank) - intel_wait_for_vblank(dev, crtc->pipe); - intel_frontbuffer_flip(dev, atomic->fb_bits); crtc->wm.cxsr_allowed = true; @@ -11660,6 +11649,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, if (!was_visible && !visible) return 0; + if (fb != old_plane_state->base.fb) + pipe_config->fb_changed = true; + turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); @@ -11676,8 +11668,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, /* must disable cxsr around plane enable/disable */ if (plane->type != DRM_PLANE_TYPE_CURSOR) { pipe_config->visible_changed = true; - if (is_crtc_enabled) - intel_crtc->atomic.wait_vblank = true; } } else if ((was_visible || visible) && intel_wm_need_update(plane, plane_state)) { @@ -11724,14 +11714,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, plane_state->rotation != BIT(DRM_ROTATE_0)) intel_crtc->atomic.disable_fbc = true; - /* -* BDW signals flip done immediately if the plane -* is disabled, even if the plane enable is already -* armed to occur at the next vblank :( -*/ - if (turn_on && IS_BROADWELL(dev)) - intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.update_fbc |= visible || mode_changed; break; case DRM_PLANE_TYPE_CURSOR: @@ -11746,12 +11728,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, if (IS_IVYBRIDGE(dev) && needs_scaling(to_intel_plane_state(plane_state)) && !needs_scaling(old_plane_state)) { - to_intel_crtc_state(crtc_state)->disable_lp_wm = true; - } else if (turn_off && !mode_changed) { - intel_crtc->atomic.wait_vblank = true; + pipe_config->disable_lp_wm = true; + } else if (turn_off && !mode_changed) intel_crtc->atomic.update_sprite_watermarks |= 1 << i; - } break; } @@ -13261,6 +13241,48 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, return ret; } +static void intel_atomic_wait_for_vblanks(struct drm_device *dev, + struct drm_i915_private *dev_priv, + unsigned crtc_mask) +{ + unsigned last_vblank_count[I915_MAX_PIPES]; + enum pipe pipe; + int ret; + + if (!crtc_mask) +
[Intel-gfx] [PATCH v2 04/14] drm/i915: Handle cdclk limits on broadwell.
As the comment indicates this can only fail gracefully when called from compute_config. Fortunately this is now what's happening, so the fixme can be removed and the DRM_ERROR downgraded. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_display.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 68f1e0f9d50d..1fcb5a379e98 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9632,14 +9632,10 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state) else cdclk = 337500; - /* -* FIXME move the cdclk caclulation to -* compute_config() so we can fail gracegully. -*/ if (cdclk > dev_priv->max_cdclk_freq) { - DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - cdclk = dev_priv->max_cdclk_freq; + DRM_INFO("requested cdclk (%d kHz) exceeds max (%d kHz)\n", +cdclk, dev_priv->max_cdclk_freq); + return -EINVAL; } to_intel_atomic_state(state)->cdclk = cdclk; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 06/14] drm/i915: Update watermark related members in the crtc_state, v2.
This removes another couple of hacks from intel_crtc->atomic, and creates proper atomic state for it. Changes since v1: - Rebase on top of wm changes. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_atomic.c | 2 ++ drivers/gpu/drm/i915/intel_display.c | 41 +--- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index c4eadbc928b7..3e390db9d22b 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -95,6 +95,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->update_pipe = false; crtc_state->disable_lp_wm = false; + crtc_state->visible_changed = false; + crtc_state->wm_changed = false; return _state->base; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 54e4f04bb427..356e3a9e1741 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4753,6 +4753,8 @@ intel_pre_disable_primary(struct drm_crtc *crtc) static void intel_post_plane_update(struct intel_crtc *crtc) { struct intel_crtc_atomic_commit *atomic = >atomic; + struct intel_crtc_state *pipe_config = + to_intel_crtc_state(crtc->base.state); struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -4761,10 +4763,9 @@ static void intel_post_plane_update(struct intel_crtc *crtc) intel_frontbuffer_flip(dev, atomic->fb_bits); - if (atomic->disable_cxsr) - crtc->wm.cxsr_allowed = true; + crtc->wm.cxsr_allowed = true; - if (crtc->atomic.update_wm_post) + if (pipe_config->wm_changed) intel_update_watermarks(>base); if (atomic->update_fbc) @@ -4781,6 +4782,8 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc_atomic_commit *atomic = >atomic; + struct intel_crtc_state *pipe_config = + to_intel_crtc_state(crtc->base.state); if (atomic->disable_fbc) intel_fbc_disable_crtc(crtc); @@ -4791,10 +4794,13 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) if (atomic->pre_disable_primary) intel_pre_disable_primary(>base); - if (atomic->disable_cxsr) { + if (pipe_config->visible_changed) { crtc->wm.cxsr_allowed = false; intel_set_memory_cxsr(dev_priv, false); } + + if (!needs_modeset(_config->base) && pipe_config->wm_changed) + intel_update_watermarks(>base); } static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask) @@ -11617,6 +11623,7 @@ static bool needs_scaling(struct intel_plane_state *state) int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) { + struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state); struct drm_crtc *crtc = crtc_state->crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_plane *plane = plane_state->plane; @@ -11663,25 +11670,18 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, plane->base.id, was_visible, visible, turn_off, turn_on, mode_changed); - if (turn_on) { - intel_crtc->atomic.update_wm_pre = true; - /* must disable cxsr around plane enable/disable */ - if (plane->type != DRM_PLANE_TYPE_CURSOR) { - intel_crtc->atomic.disable_cxsr = true; - /* to potentially re-enable cxsr */ - intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.update_wm_post = true; - } - } else if (turn_off) { - intel_crtc->atomic.update_wm_post = true; + if (turn_on || turn_off) { + pipe_config->wm_changed = true; + /* must disable cxsr around plane enable/disable */ if (plane->type != DRM_PLANE_TYPE_CURSOR) { + pipe_config->visible_changed = true; if (is_crtc_enabled) intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.disable_cxsr = true; } - } else if (intel_wm_need_update(plane, plane_state)) { - intel_crtc->atomic.update_wm_pre = true; + } else if ((was_visible || visible) && + intel_wm_need_update(plane, plane_state)) { +
[Intel-gfx] [PATCH v2 05/14] drm/i915/bxt: Use the bypass frequency if there are no active pipes.
Now that pixel clock is set to 0 when there are no active pipes it's easy to set the bypass frequency for this case. Signed-off-by: Maarten Lankhorst--- 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 1fcb5a379e98..54e4f04bb427 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5917,7 +5917,6 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, /* * FIXME: * - remove the guardband, it's not needed on BXT -* - set 19.2MHz bypass frequency if there are no active pipes */ if (max_pixclk > 576000*9/10) return 624000; @@ -5927,8 +5926,10 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv, return 384000; else if (max_pixclk > 144000*9/10) return 288000; - else + else if (max_pixclk) return 144000; + else + return 19200; } /* Compute the max pixel clock for new configuration. Uses atomic state if -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 13/14] drm/i915/skl: Update watermarks before the crtc is disabled.
On skylake some of the registers are only writable when the correct power wells are enabled. Because of this watermarks have to be updated before the crtc turns off, or you get unclaimed register read and write warnings. This patch needs to be modified slightly to apply to -fixes. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181 Signed-off-by: Maarten LankhorstCc: sta...@vger.kernel.org Cc: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecd3169b45ef..304e1028c9a4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4758,7 +4758,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) crtc->wm.cxsr_allowed = true; - if (pipe_config->wm_changed) + if (pipe_config->wm_changed && pipe_config->base.active) intel_update_watermarks(>base); if (old_pri_state) { @@ -13327,6 +13327,9 @@ static int intel_atomic_commit(struct drm_device *dev, dev_priv->display.crtc_disable(crtc); intel_crtc->active = false; intel_disable_shared_dpll(intel_crtc); + + if (!crtc->state->active) + intel_update_watermarks(crtc); } } -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 10/14] drm/i915: Remove update_sprite_watermarks.
Commit 791a32be6eb2 ("drm/i915: Drop intel_update_sprite_watermarks") removes the use of this variable, but forgot to remove it. Cc: Matt RoperSigned-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 6 +- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b1d6ce80daaf..77fe1d75404b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11632,7 +11632,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, struct intel_plane_state *old_plane_state = to_intel_plane_state(plane->state); int idx = intel_crtc->base.base.id, ret; - int i = drm_plane_index(plane); bool mode_changed = needs_modeset(crtc_state); bool was_crtc_enabled = crtc->state->active; bool is_crtc_enabled = crtc_state->active; @@ -11726,11 +11725,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, */ if (IS_IVYBRIDGE(dev) && needs_scaling(to_intel_plane_state(plane_state)) && - !needs_scaling(old_plane_state)) { + !needs_scaling(old_plane_state)) pipe_config->disable_lp_wm = true; - } else if (turn_off && !mode_changed) - intel_crtc->atomic.update_sprite_watermarks |= - 1 << i; break; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8d86627069e5..dc024c27ca1e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -542,7 +542,6 @@ struct intel_crtc_atomic_commit { unsigned fb_bits; bool update_fbc; bool post_enable_primary; - unsigned update_sprite_watermarks; }; struct intel_crtc { -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 09/14] drm/i915: Remove atomic.pre_disable_primary.
This can be derived from the atomic state in pre_plane_update, which makes it more clear when it's supposed to be called. Signed-off-by: Maarten Lankhorst--- drivers/gpu/drm/i915/intel_display.c | 28 drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 58074f4adca2..b1d6ce80daaf 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4766,26 +4766,40 @@ static void intel_post_plane_update(struct intel_crtc *crtc) memset(atomic, 0, sizeof(*atomic)); } -static void intel_pre_plane_update(struct intel_crtc *crtc) +static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state) { + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc_atomic_commit *atomic = >atomic; struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->base.state); + struct drm_atomic_state *old_state = old_crtc_state->base.state; + struct drm_plane *primary = crtc->base.primary; + struct drm_plane_state *old_pri_state = + drm_atomic_get_existing_plane_state(old_state, primary); + bool modeset = needs_modeset(_config->base); if (atomic->disable_fbc) intel_fbc_disable_crtc(crtc); - if (atomic->pre_disable_primary) - intel_pre_disable_primary(>base); + if (old_pri_state) { + struct intel_plane_state *primary_state = + to_intel_plane_state(primary->state); + struct intel_plane_state *old_primary_state = + to_intel_plane_state(old_pri_state); + + if (old_primary_state->visible && + (modeset || !primary_state->visible)) + intel_pre_disable_primary(>base); + } if (pipe_config->visible_changed) { crtc->wm.cxsr_allowed = false; intel_set_memory_cxsr(dev_priv, false); } - if (!needs_modeset(_config->base) && pipe_config->wm_changed) + if (!modeset && pipe_config->wm_changed) intel_update_watermarks(>base); } @@ -11677,7 +11691,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, switch (plane->type) { case DRM_PLANE_TYPE_PRIMARY: - intel_crtc->atomic.pre_disable_primary = turn_off; intel_crtc->atomic.post_enable_primary = turn_on; if (turn_off) @@ -13318,7 +13331,7 @@ static int intel_atomic_commit(struct drm_device *dev, if (!needs_modeset(crtc->state)) continue; - intel_pre_plane_update(intel_crtc); + intel_pre_plane_update(to_intel_crtc_state(crtc_state)); if (crtc_state->active) { intel_crtc_disable_planes(crtc, crtc_state->plane_mask); @@ -13341,7 +13354,6 @@ static int intel_atomic_commit(struct drm_device *dev, /* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state(state, crtc, crtc_state, i) { - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); @@ -13361,7 +13373,7 @@ static int intel_atomic_commit(struct drm_device *dev, } if (!modeset) - intel_pre_plane_update(intel_crtc); + intel_pre_plane_update(to_intel_crtc_state(crtc_state)); if (crtc->state->active && (crtc->state->planes_changed || update_pipe)) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ce5f10fc92aa..8d86627069e5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -537,7 +537,6 @@ struct intel_mmio_flip { struct intel_crtc_atomic_commit { /* Sleepable operations to perform before commit */ bool disable_fbc; - bool pre_disable_primary; /* Sleepable operations to perform after commit */ unsigned fb_bits; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device.
So far, the i915 driver and some other drivers set it to the drm_device, which doesn't allow one to know which DP a given aux channel is related to. Changing this to be the drm_connector provides proper nesting, still allowing one to get the drm_device from it. Some drivers already set it to the drm_connector. This also removes the need to add a sysfs link for the i2c device under the connector, as it will already be there. Signed-off-by: Rafael Antognolli--- drivers/gpu/drm/i915/intel_dp.c | 19 ++- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8287df4..7aacc08 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1078,36 +1078,21 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector) intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10; intel_dp->aux.name = name; - intel_dp->aux.dev = dev->dev; + intel_dp->aux.dev = connector->base.kdev; intel_dp->aux.transfer = intel_dp_aux_transfer; DRM_DEBUG_KMS("registering %s bus for %s\n", name, connector->base.kdev->kobj.name); ret = drm_dp_aux_register(_dp->aux); - if (ret < 0) { + if (ret < 0) DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n", name, ret); - return; - } - - ret = sysfs_create_link(>base.kdev->kobj, - _dp->aux.ddc.dev.kobj, - intel_dp->aux.ddc.dev.kobj.name); - if (ret < 0) { - DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret); - drm_dp_aux_unregister(_dp->aux); - } } static void intel_dp_connector_unregister(struct intel_connector *intel_connector) { - struct intel_dp *intel_dp = intel_attached_dp(_connector->base); - - if (!intel_connector->mst_port) - sysfs_remove_link(_connector->base.kdev->kobj, - intel_dp->aux.ddc.dev.kobj.name); intel_connector_unregister(intel_connector); } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
This module is heavily based on i2c-dev. Once loaded, it provides one dev node per DP AUX channel, named drm_dp_auxN, where N is an integer. It's possible to know which connector owns this aux channel by looking at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if the connector device pointer was correctly set in the aux helper struct. Two main operations are provided on the registers read and write. The address of the register to be read or written is given using lseek. The seek position is updated upon read or write. v2: - lseek is used to select the register to read/write - read/write are used instead of ioctl - no blocking_notifier is used, just a direct callback v3: - use drm_dp_aux_dev prefix for public functions - chardev is named drm_dp_auxN - read/write don't allocate a buffer anymore, and transfer up to 16 bytes a time - remove notifier list from the implementation - option on menuconfig is now a boolean - add inline stub functions to avoid breakage when this option is disabled v4: - fix build system changes - actually disable this module when not selected. v5: - Use kref to avoid device closing while still in use - Don't use list, use an idr for storing aux_dev - Remove "connector" attribute - set aux.dev to the connector drm_connector device, instead of drm_device v6: - Use atomic_t for usage count - Use a mutex instead of spinlock for idr lock - Destroy chardev immediately on unregister - other minor suggestions from Ville v7: - style fixes - error handling fixes v8: - more error handling fixes Signed-off-by: Rafael Antognolli--- drivers/gpu/drm/Kconfig | 8 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 373 +++ drivers/gpu/drm/drm_dp_helper.c | 16 +- include/drm/drm_dp_aux_dev.h | 50 ++ 5 files changed, 447 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c create mode 100644 include/drm/drm_dp_aux_dev.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c4bf9a1..daefcce 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -25,6 +25,14 @@ config DRM_MIPI_DSI bool depends on DRM +config DRM_DP_AUX_CHARDEV + bool "DRM DP AUX Interface" + depends on DRM + help + Choose this option to enable a /dev/drm_dp_auxN node that allows to + read and write values to arbitrary DPCD registers on the DP aux + channel. + config DRM_KMS_HELPER tristate depends on DRM diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 1e9ff4c..e48ec8f 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c new file mode 100644 index 000..0269556 --- /dev/null +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -0,0 +1,373 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Rafael Antognolli + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct drm_dp_aux_dev { + unsigned index; + struct drm_dp_aux *aux; + struct device *dev; + struct kref refcount; + atomic_t usecount; +}; + +#define DRM_AUX_MINORS 256 +#define AUX_MAX_OFFSET (1 << 20)
[Intel-gfx] [PATCH v8 0/2] Add drm_dp_aux chardev support.
This series implement support to a drm_dp_aux chardev that allows reading and writing an arbitrary amount of bytes to arbitrary dpcd register addresses using regular read, write and lseek operations. Rafael Antognolli (2): drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers. drm/dp: Set aux.dev to the drm_connector device, instead of drm_device. drivers/gpu/drm/Kconfig | 8 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 373 +++ drivers/gpu/drm/drm_dp_helper.c | 16 +- drivers/gpu/drm/i915/intel_dp.c | 19 +- include/drm/drm_dp_aux_dev.h | 50 ++ 6 files changed, 449 insertions(+), 18 deletions(-) create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c create mode 100644 include/drm/drm_dp_aux_dev.h -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Fix build issue with 15_10 config with amdgpu
From: Jim BishSigned-off-by: Jim Bish --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index 0587bb2..15dd6fd 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -3139,6 +3139,7 @@ static int dce_v10_0_resume(void *handle) dce_v10_0_pageflip_interrupt_init(adev); return ret; +} static bool dce_v10_0_is_idle(void *handle) { -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Allow unready gpu to be reset on gen8
> From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com] > Chris Wilsonwrites: > > So you are saying that's there no bugzilla for this... :-p > > Bugzilla fairy might surprise us after a good weekend rest. https://bugs.freedesktop.org/show_bug.cgi?id=92774 Regards, Tomi - Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Hardlock problems when switching monitor channels and questions (Skylake)
Hi everyone. I've been some hardlock problems when switching from the HDMI connection of my monitor (connected to another PC) and the displayport (connected to the problematic PC), several times a week at least. In an effort to narrow down the problem I've been tried looking at drm modeset debug information, as well try catch something with netconsole. Neither have pointed me the root cause yet, but i do have some observations that I cannot put into place yet and would like some feedback on prior to opening a bug report. My mainbord is: Asrock Fatal1ty Z170 Gaming-ITX/ac My CPU is: Intel i7 6700 My monitor is: Samsung U24E850, equiped with both displayport 1.2 and HDMI 2.0 Kernel version: 4.3-RC7 The display connections on the mainbord are: - DP 1.2 - HDMI 1.4 - HDMI 2.0 via DP 1.2 (recognized as DP by driver) I notice some funny things I would like some feedback on. 1. The EDID retrieval doesn't always succeed, i've noticed this sometimes by running "xrandr" after starting up. I've also noticed it after rebooting from a hard lock that it goes into 1024x768 resolution instead of 3840x2160, it takes another reboot to get it right. As if it relies on the resolution the boot used? How to troubleshoot this? 2. I notice that the driver thinks there are 3 HDMI connections, 2 of which seem to share the same hotplug pin as the 2 DP connections, what is going here? Could this cause trouble? The fake HDMI connections seem always to fail edid retrieval, but I'm not a 100% sure if this also happens in the hardlock situation (no evidence yet). 3. Switching between HDMI and DP channel on my monitor sometimes results in a hotplug event and sometimes it does not? Any idea why this happens? I'm curious about this because the hardlocks occur after spending a few hours on the HDMI channel typically. Anything I should be looking for before going down the route of a bug report? Sincerely, Maarten. P.S. I'm not subscribed to this mailinglist, so please CC me. -- Far away from the primal instinct, the song seems to fade away, the river get wider between your thoughts and the things we do and say. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fall back to zero vswing/preemph if the sink doesn't like the last good values
On Fri, 30 Oct 2015, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > My Lenovo STM STDP3100 miniDP->VGA dongle doesn't seem to like it when > we try to start link training with non-zero vswing/preemphasis. So when > the initial link training DPCD write fails, retry it with zero values. > > Fixes a bunch of errors like so: > [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training > > Cc: Mika Kahola > Cc: Sivakumar Thulasimani > Cc: Ander Conselvan de Oliveira > Fixes: 5fa836a9d859 ("drm/i915: DP link training optimization") > Signed-off-by: Ville Syrjälä Possibly Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393 > --- > drivers/gpu/drm/i915/intel_dp.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ba4cbf5..9529a6e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3750,10 +3750,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp > *intel_dp) > > DP |= DP_PORT_EN; > > +again: > /* clock recovery */ > if (!intel_dp_reset_link_train(intel_dp, , > DP_TRAINING_PATTERN_1 | > DP_LINK_SCRAMBLING_DISABLE)) { > + if (intel_dp->train_set_valid) { > + DRM_DEBUG_KMS("Sink rejected link training request, > trying again with zero values\n"); > + intel_dp->train_set_valid = false; > + goto again; > + } > + > DRM_ERROR("failed to enable link training\n"); > return; > } -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add quirk to enable backlight on Dell Chromebook 11 (2015)
On Fri, 30 Oct 2015, Clint Taylorwrote: > On 10/30/2015 05:50 AM, Jani Nikula wrote: >> Reported-by: Keith Webb >> Suggested-by: Keith Webb >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106671 >> Signed-off-by: Jani Nikula >> --- >> 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 cfce7ea89d08..ec159df836a6 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -14809,6 +14809,9 @@ static struct intel_quirk intel_quirks[] = { >> >> /* Dell Chromebook 11 */ >> { 0x0a06, 0x1028, 0x0a35, quirk_backlight_present }, >> + >> +/* Dell Chromebook 11 (2015 version) */ >> +{ 0x0a16, 0x1028, 0x0a35, quirk_backlight_present }, >> }; >> >> static void intel_init_quirks(struct drm_device *dev) >> > > Reviewed-by: Clint Taylor Pushed to drm-intel-next-fixes, thanks for the review. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/8] kms_force_connector: use comparison macros to make debug output clearer
On 2 November 2015 at 11:48, Thomas Woodwrote: > Signed-off-by: Thomas Wood > --- > tests/kms_force_connector.c | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c > index 7485ca8..4ba1e0b 100644 > --- a/tests/kms_force_connector.c > +++ b/tests/kms_force_connector.c > @@ -27,8 +27,9 @@ > IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work" > " correctly."); > > -#define CHECK_MODE(m, h, w, r) igt_assert(m.hdisplay == h && m.vdisplay == w > \ > - && m.vrefresh == r) > +#define CHECK_MODE(m, h, w, r) \ > + igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \ > + igt_assert_eq(m.vrefresh, r); > > igt_main > { > @@ -63,8 +64,8 @@ igt_main > /* force the connector on and check the reported values */ > kmstest_force_connector(drm_fd, vga_connector, > FORCE_CONNECTOR_ON); > temp = drmModeGetConnector(drm_fd, > vga_connector->connector_id); > - igt_assert(temp->connection == DRM_MODE_CONNECTED); > - igt_assert(temp->count_modes > 0); > + igt_assert_eq(temp->connection, DRM_MODE_CONNECTED); > + igt_assert_lt(0, temp->count_modes); > drmModeFreeConnector(temp); > > /* attempt to use the display */ > @@ -77,15 +78,15 @@ igt_main > kmstest_force_connector(drm_fd, vga_connector, > FORCE_CONNECTOR_OFF); > temp = drmModeGetConnector(drm_fd, > vga_connector->connector_id); > - igt_assert(temp->connection == DRM_MODE_DISCONNECTED); > - igt_assert(temp->count_modes == 0); > + igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED); > + igt_assert_lt(0, temp->count_modes); This should have been igt_assert_eq. > drmModeFreeConnector(temp); > > /* check that the previous state is restored */ > kmstest_force_connector(drm_fd, vga_connector, > FORCE_CONNECTOR_UNSPECIFIED); > temp = drmModeGetConnector(drm_fd, > vga_connector->connector_id); > - igt_assert(temp->connection == vga_connector->connection); > + igt_assert_eq(temp->connection, vga_connector->connection); > drmModeFreeConnector(temp); > } > > @@ -115,7 +116,7 @@ igt_main > temp = drmModeGetConnector(drm_fd, > vga_connector->connector_id); > /* the connector should now have the same number of modes that > * it started with */ > - igt_assert(temp->count_modes == start_n_modes); > + igt_assert_eq(temp->count_modes, start_n_modes); > drmModeFreeConnector(temp); > > kmstest_force_connector(drm_fd, vga_connector, > -- > 1.9.1 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/5] tests/kms_force_connector: free the display struct when no longer needed
Signed-off-by: Thomas Wood--- tests/kms_force_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 7485ca8..f48b187 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -36,7 +36,6 @@ igt_main int drm_fd = 0; drmModeRes *res; drmModeConnector *vga_connector = NULL, *temp; - igt_display_t display; int start_n_modes; igt_fixture { @@ -60,6 +59,8 @@ igt_main } igt_subtest("force-connector-state") { + igt_display_t display; + /* force the connector on and check the reported values */ kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); @@ -71,6 +72,7 @@ igt_main kmstest_set_vt_graphics_mode(); igt_display_init(, drm_fd); igt_display_commit(); + igt_display_fini(); /* force the connector off */ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/5] tests/kms_force_connector: skip if the required connector is connected
Signed-off-by: Thomas Wood--- tests/kms_force_connector.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index f48b187..1e5ec6f 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -56,6 +56,7 @@ igt_main } igt_require(vga_connector); + igt_skip_on(vga_connector->connection == DRM_MODE_CONNECTED); } igt_subtest("force-connector-state") { -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On 11/2/2015 6:49 PM, Kumar, Shobhit wrote: On 11/02/2015 06:40 PM, Jani Nikula wrote: On Mon, 02 Nov 2015, Shobhit Kumarwrote: SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? As per my discussion with gop team, this has been enabled in main stream for quite sometime including VLV, CHT, BDW, SKL+ and is common for GOP/VBIOS across chrome/windows/android. So yes I think we can universally trust as of now. This has been added since IVB timeframe and should be part of VBT spec. but i just encountered an issue in Android Charging OS where there is no modeset and is using the displays enabled by GOP/VBIOS. This patch might break such expectations. Regards Shobhit BR, Jani. Cc: Ville Syrjälä Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i)(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i)(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; +/* + * check if the pre-os intialized the display + * There is SWF18 scratchpad register defined which is set by the + * pre-os which can be used by the OS drivers to check the status + */ +if ((I915_READ(GEN6_SWF18) & 0x00) == 0) +goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Add aux plane verification in addFB2
On 11/2/2015 3:20 PM, Vandana Kannan wrote: For render compression, userspace passes aux stride and offset values as an additional entry in the fb structure. This should not be treated as garbage and discarded as data belonging to no plane. This patch introduces a check related to AUX plane to support the scenario of render compression. Suggested-by: Daniel VetterSigned-off-by: Vandana Kannan --- drivers/gpu/drm/drm_crtc.c | 16 +++- drivers/gpu/drm/drm_ioctl.c | 3 +++ include/drm/drm_crtc.h | 3 +++ include/uapi/drm/drm.h | 1 + include/uapi/drm/drm_mode.h | 1 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 24c5434..7dbc0f0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3204,6 +3204,13 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } } + if (r->flags & DRM_MODE_FB_AUX_PLANE) { + if (num_planes == 4) how can num_planes be 4 since the max it can get seems to be 3. regards, Sivakumar + return -EINVAL; + + num_planes++; + } + for (i = num_planes; i < 4; i++) { if (r->modifier[i]) { DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i); @@ -3242,7 +3249,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret; - if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) { + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS | + DRM_MODE_FB_AUX_PLANE)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL); } @@ -3264,6 +3272,12 @@ internal_framebuffer_create(struct drm_device *dev, return ERR_PTR(-EINVAL); } + if (r->flags & DRM_MODE_FB_AUX_PLANE && + !dev->mode_config.allow_aux_plane) { + DRM_DEBUG_KMS("driver does not support render compression\n"); + return ERR_PTR(-EINVAL); + } + ret = framebuffer_check(r); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 8ce2a0c..ee00782 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_ADDFB2_MODIFIERS: req->value = dev->mode_config.allow_fb_modifiers; break; + case DRM_CAP_RENDER_COMPRESSION: + req->value = dev->mode_config.allow_aux_plane; + break; default: return -EINVAL; } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3f0c690..a5a9da2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1152,6 +1152,9 @@ struct drm_mode_config { /* whether the driver supports fb modifiers */ bool allow_fb_modifiers; + /* whether the driver supports render compression */ + bool allow_aux_plane; + /* cursor size */ uint32_t cursor_width, cursor_height; }; diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 3801584..0834bf7 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -631,6 +631,7 @@ struct drm_gem_open { #define DRM_CAP_CURSOR_WIDTH 0x8 #define DRM_CAP_CURSOR_HEIGHT 0x9 #define DRM_CAP_ADDFB2_MODIFIERS 0x10 +#define DRM_CAP_RENDER_COMPRESSION 0x11 /** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 6c11ca4..de59ace 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd { #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_AUX_PLANE (1<<2) /* for compressed buffer */ struct drm_mode_fb_cmd2 { __u32 fb_id; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 5/5] tests/kms_force_connector: use comparison macros
Use the comparison macros to make debug output clearer. v2: fix incorrect comparison Signed-off-by: Thomas Wood--- tests/kms_force_connector.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 2cb149a..f34f21a 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -27,8 +27,9 @@ IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work" " correctly."); -#define CHECK_MODE(m, h, w, r) igt_assert(m.hdisplay == h && m.vdisplay == w \ - && m.vrefresh == r) +#define CHECK_MODE(m, h, w, r) \ + igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \ + igt_assert_eq(m.vrefresh, r); static void __attribute__((noreturn)) reset_connectors() { @@ -107,8 +108,8 @@ int main(int argc, char **argv) /* force the connector on and check the reported values */ kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == DRM_MODE_CONNECTED); - igt_assert(temp->count_modes > 0); + igt_assert_eq(temp->connection, DRM_MODE_CONNECTED); + igt_assert_lt(0, temp->count_modes); drmModeFreeConnector(temp); /* attempt to use the display */ @@ -122,15 +123,15 @@ int main(int argc, char **argv) kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_OFF); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == DRM_MODE_DISCONNECTED); - igt_assert(temp->count_modes == 0); + igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED); + igt_assert_eq(0, temp->count_modes); drmModeFreeConnector(temp); /* check that the previous state is restored */ kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_UNSPECIFIED); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == vga_connector->connection); + igt_assert_eq(temp->connection, vga_connector->connection); drmModeFreeConnector(temp); } @@ -160,7 +161,7 @@ int main(int argc, char **argv) temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); /* the connector should now have the same number of modes that * it started with */ - igt_assert(temp->count_modes == start_n_modes); + igt_assert_eq(temp->count_modes, start_n_modes); drmModeFreeConnector(temp); kmstest_force_connector(drm_fd, vga_connector, -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/5] tests/kms_force_connector: add an option to reset connector force states
Signed-off-by: Thomas Wood--- tests/kms_force_connector.c | 44 +++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 1e5ec6f..2cb149a 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -30,13 +30,55 @@ IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work" #define CHECK_MODE(m, h, w, r) igt_assert(m.hdisplay == h && m.vdisplay == w \ && m.vrefresh == r) -igt_main +static void __attribute__((noreturn)) reset_connectors() +{ + int drm_fd = 0; + drmModeRes *res; + drmModeConnector *connector = NULL; + + drm_fd = drm_open_driver_master(DRIVER_INTEL); + res = drmModeGetResources(drm_fd); + + for (int i = 0; i < res->count_connectors; i++) { + + connector = drmModeGetConnector(drm_fd, res->connectors[i]); + + kmstest_force_connector(drm_fd, connector, + FORCE_CONNECTOR_UNSPECIFIED); + + drmModeFreeConnector(connector); + } + + exit(0); +} + +static int opt_handler(int opt, int opt_index, void *data) +{ + switch (opt) { + case 'r': + reset_connectors(); + break; + } + + return 0; +} + +int main(int argc, char **argv) { /* force the VGA output and test that it worked */ int drm_fd = 0; drmModeRes *res; drmModeConnector *vga_connector = NULL, *temp; int start_n_modes; + struct option long_opts[] = { + {"reset", 0, 0, 'r'}, + {0, 0, 0, 0} + }; + const char *help_str = + " --reset\t\tReset all connector force states.\n"; + + igt_subtest_init_parse_opts(, argv, "", long_opts, help_str, + opt_handler, NULL); igt_fixture { drm_fd = drm_open_driver_master(DRIVER_INTEL); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/5] lib: add documentation for igt_display_init/fini
Signed-off-by: Thomas Wood--- lib/igt_kms.c | 16 1 file changed, 16 insertions(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 51d735d..878e1fb 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -962,6 +962,15 @@ static int get_drm_plane_type(int drm_fd, uint32_t plane_id) return DRM_PLANE_TYPE_OVERLAY; } +/** + * igt_display_init: + * @display: a pointer to an #igt_display_t structure + * @drm_fd: a drm file descriptor + * + * Initialize @display and allocate the various resources required. Use + * #igt_display_fini to release the resources when they are no longer required. + * + */ void igt_display_init(igt_display_t *display, int drm_fd) { drmModeRes *resources; @@ -1160,6 +1169,13 @@ static void igt_output_fini(igt_output_t *output) free(output->name); } +/** + * igt_display_fini: + * @display: a pointer to an #igt_display_t structure + * + * Release any resources associated with @display. This does not free @display + * itself. + */ void igt_display_fini(igt_display_t *display) { int i; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 03:28:22PM +, Chris Wilson wrote: > That should keep the worker alive for a further 10 jiffies, hopefully > long enough for the next wait to occur. The cost is that it keeps the > interrupt asserted (and to avoid that requires a little rearrangment and > probably a dedicated kthread for each ring). Slightly better version that avoids keeping the interrupt active when not required: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8d1e0a5e19a1..3e810c9ea3cb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1148,7 +1148,7 @@ static int __i915_spin_request(struct drm_i915_gem_request *req) { unsigned long timeout; - if (i915_gem_request_get_ring(req)->irq_refcount) + if (waitqueue_active(_gem_request_get_ring(req)->irq_queue)) return -EBUSY; if (req->ring->seqno_barrier) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 21017daecb90..9ed063a85c05 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -68,9 +68,11 @@ static void intel_engine_irq_wakeup(struct work_struct *work) { struct intel_engine_cs *engine = container_of(work, struct intel_engine_cs, irq_work); - const bool noirq = !__irq_enable(engine); + bool noirq; DEFINE_WAIT(wait); +restart: + noirq = !__irq_enable(engine); for (;;) { struct timer_list timer; struct drm_i915_gem_request *request; @@ -142,9 +144,14 @@ static void intel_engine_irq_wakeup(struct work_struct *work) destroy_timer_on_stack(); } } - finish_wait(>irq_queue, ); if (!noirq) engine->irq_put(engine); + + prepare_to_wait(>irq_queue, , TASK_UNINTERRUPTIBLE); + if (schedule_timeout(msecs_to_jiffies_timeout(20)) > 0) + goto restart; + + finish_wait(>irq_queue, ); } void intel_engine_init_wakeups(struct intel_engine_cs *engine) @@ -163,8 +170,10 @@ void intel_engine_add_wakeup(struct drm_i915_gem_request *request) struct rb_node **p, *parent; bool first; - if (RB_EMPTY_ROOT(>irq_requests)) + if (RB_EMPTY_ROOT(>irq_requests)) { schedule_work(>irq_work); + wake_up_all(>irq_queue); + } init_waitqueue_head(>wait); -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Hey, Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > From: Ville Syrjälä> > This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9. > > commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully interruptible.") > breaks GPU reset on gen3/4 machines. Go back to to non-interruptible. More context please? I don't think reverting is the way to go here. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915/dma: enforce pr_ consistency
On Sat, 31 Oct 2015, Ioan-Adrian Ratiuwrote: > One branch of the if clause uses pr_info, the other pr_err; change > the 'false' branch to also use pr_info. This minor oversight has gone > unfixed since the initial vga_switcheroo implementation in 6a9ee8af. > > Signed-off-by: Ioan-Adrian Ratiu Pushed to drm-intel-next-queued, thanks for the patch. BR, Jani. > --- > drivers/gpu/drm/i915/i915_dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 2336af9..b23f465 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -338,7 +338,7 @@ static void i915_switcheroo_set_state(struct pci_dev > *pdev, enum vga_switcheroo_ > i915_resume_switcheroo(dev); > dev->switch_power_state = DRM_SWITCH_POWER_ON; > } else { > - pr_err("switched off\n"); > + pr_info("switched off\n"); > dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; > i915_suspend_switcheroo(dev, pmm); > dev->switch_power_state = DRM_SWITCH_POWER_OFF; -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)
On 31/10/15 01:45, Vivek Kasireddy wrote: Hi Tvrtko, On Fri, 30 Oct 2015 10:22:08 + Tvrtko Ursulinwrote: On 30/10/15 01:44, Vivek Kasireddy wrote: The main goal of this subtest is to trigger the following warning in the function i915_gem_object_get_fence(): if (WARN_ON(!obj->map_and_fenceable)) To trigger this warning, the subtest first creates a Y-tiled object and an associated framebuffer with the Y-fb modifier. Furthermore, to prevent the map_and_fenceable from being set, we make sure that the object does not have a normal VMA by refraining from rendering to the object and by setting the rotation property upfront before calling commit. v2: Do not call paint_squares and just use one output. v3: Convert an if condition to igt_require and move the plane rotation requirement further up before the fb allocation. v4: After setting rotation to 90 and committing, change the rotation to 0 and commit once more. This is to test if the i915 driver hits any warnings while pinning and unpinning an object that has both normal and rotated views. v5: - Add another subtest to toggle the order of rotation - Exhaustively test the i915 driver's pinning and unpinning code paths for any fence leaks by iterating until MAX available fences. Cc: Tvrtko Ursulin Signed-off-by: Vivek Kasireddy --- tests/kms_rotation_crc.c | 84 1 file changed, 84 insertions(+) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index cc9847e..34f8150 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) igt_require_f(valid_tests, "no valid crtc/connector combinations found\n"); } +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type, + int toggle) +{ + igt_display_t *display = >display; + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + uint32_t format = DRM_FORMAT_XRGB; + int bpp = igt_drm_format_to_bpp(format); + enum igt_commit_style commit = COMMIT_LEGACY; + int fd = data->gfx_fd; + igt_output_t *output = >outputs[0]; + igt_plane_t *plane; + drmModeModeInfo *mode; + unsigned int stride, size, w, h; + uint32_t gem_handle; + int num_fences = gem_available_fences(fd); + int i, ret; + + igt_require(output != NULL && output->valid == true); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + + if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + for (stride = 512; stride < (w * bpp / 8); stride *= 2) + ; + for (size = 1024*1024; size < stride * h; size *= 2) + ; + + gem_handle = gem_create(fd, size); + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride); + igt_assert(ret == 0); + + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, + >fb.fb_id)); + data->fb.width = w; + data->fb.height = h; + data->fb.gem_handle = gem_handle; + + igt_plane_set_fb(plane, NULL); + igt_display_commit(display); + + igt_plane_set_fb(plane, >fb); + + for (i = 0; i < num_fences + 1; i++) { + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 : IGT_ROTATION_90); + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, +DRM_MODE_OBJECT_PLANE, +plane->rotation_property, +plane->rotation); + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 : IGT_ROTATION_0); + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, +DRM_MODE_OBJECT_PLANE, +plane->rotation_property, +plane->rotation); + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + } It manages to exhaust the fences with only one object? I was expecting that multiple objects will be required since if it is only one how come it allocates more than one fence register? Before I sent out this version, I did try with two objects to see if it triggers any WARNs but it didn't. I am not sure if I did it right though,
Re: [Intel-gfx] [PATCH] drm/i915: make A0 wa's applied to A1
Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Friday, October 30, 2015 4:11 PM > To: Gore, Tim > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: make A0 wa's applied to A1 > > On Mon, Oct 26, 2015 at 10:48:58AM +, tim.g...@intel.com wrote: > > From: Tim Gore> > > > Since A1 chips use the same GPU as A0, they need all the same wa's in > > the i915 driver. Update some conditionals to do this. > > Neither summary nor commit message mentions that this is for bxt. Please > fix. > -Daniel > This seems to have been merged. Is it possible for me to fix the commit message retrospectively (other than in my local repo)? Tim > > > > Signed-off-by: Tim Gore > > --- > > drivers/gpu/drm/i915/intel_guc_loader.c | 2 +- > > drivers/gpu/drm/i915/intel_lrc.c| 8 > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > > b/drivers/gpu/drm/i915/intel_guc_loader.c > > index c0281df..6ec7b23 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > @@ -309,7 +309,7 @@ static int guc_ucode_xfer(struct drm_i915_private > > *dev_priv) > > > > /* WaDisableMinuteIaClockGating:skl,bxt */ > > if (IS_SKL_REVID(dev, 0, SKL_REVID_B0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) { > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) { > > I915_WRITE(GUC_SHIM_CONTROL, > (I915_READ(GUC_SHIM_CONTROL) & > > > ~GUC_ENABLE_MIA_CLOCK_GATING)); > > } > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index 3265427..971d3f2 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -285,7 +285,7 @@ static bool disable_lite_restore_wa(struct > intel_engine_cs *ring) > > struct drm_device *dev = ring->dev; > > > > return (IS_SKL_REVID(dev, 0, SKL_REVID_B0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) && > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) && > >(ring->id == VCS || ring->id == VCS2); } > > > > @@ -1315,7 +1315,7 @@ static int gen9_init_indirectctx_bb(struct > > intel_engine_cs *ring, > > > > /* WaDisableCtxRestoreArbitration:skl,bxt */ > > if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) > > wa_ctx_emit(batch, index, MI_ARB_ON_OFF | > MI_ARB_DISABLE); > > > > /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */ @@ - > 1341,7 > > +1341,7 @@ static int gen9_init_perctx_bb(struct intel_engine_cs > > *ring, > > > > /* > WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */ > > if (IS_SKL_REVID(dev, 0, SKL_REVID_B0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) { > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) { > > wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1)); > > wa_ctx_emit(batch, index, > GEN9_SLICE_COMMON_ECO_CHICKEN0); > > wa_ctx_emit(batch, index, > > @@ -1351,7 +1351,7 @@ static int gen9_init_perctx_bb(struct > > intel_engine_cs *ring, > > > > /* WaDisableCtxRestoreArbitration:skl,bxt */ > > if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) > > wa_ctx_emit(batch, index, MI_ARB_ON_OFF | > MI_ARB_ENABLE); > > > > wa_ctx_emit(batch, index, MI_BATCH_BUFFER_END); diff --git > > a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 0fb0459..911f837 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4691,7 +4691,7 @@ static void gen9_enable_rc6(struct drm_device > *dev) > > "on" : "off"); > > /* WaRsUseTimeoutMode */ > > if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) || > > - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) { > > + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) { > > I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */ > > I915_WRITE(GEN6_RC_CONTROL, > GEN6_RC_CTL_HW_ENABLE | > >GEN7_RC_CTL_TO_MODE | > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 02a5bb0..368d78b 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1099,11 +1099,11 @@ static int bxt_init_workarounds(struct > > intel_engine_cs *ring) > > > > /* WaStoreMultiplePTEenable:bxt */ > > /* This is a requirement according to Hardware specification */ > > - if (IS_BXT_REVID(dev, 0, BXT_REVID_A0)) > > +
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 05:39:54AM +, Gong, Zhipeng wrote: > Chris- > > The patch cannot be applied on the latest drm-intel-nightly directly. > I modified it a little bit to make it applied. > The patch can help much in HSW, but a little bit in BDW. > The test is to transcode 26 streams, which creates 244 threads. > > CPU util | w/o patch | w/ patch > -- > HSW async 1 | 102% | 61% > HSW async 5 | 114% | 46% > BDW async 1 | 116% | 116% > BDW async 5 | 111% | 107% Could I get the perf report for the kernel time? One aspect that I find hard to believe is that it is not the execbuf/mutex-contention that is the ratelimiting step. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Add aux plane verification in addFB2
For render compression, userspace passes aux stride and offset values as an additional entry in the fb structure. This should not be treated as garbage and discarded as data belonging to no plane. This patch introduces a check related to AUX plane to support the scenario of render compression. Suggested-by: Daniel VetterSigned-off-by: Vandana Kannan --- drivers/gpu/drm/drm_crtc.c | 16 +++- drivers/gpu/drm/drm_ioctl.c | 3 +++ include/drm/drm_crtc.h | 3 +++ include/uapi/drm/drm.h | 1 + include/uapi/drm/drm_mode.h | 1 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 24c5434..7dbc0f0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3204,6 +3204,13 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } } + if (r->flags & DRM_MODE_FB_AUX_PLANE) { + if (num_planes == 4) + return -EINVAL; + + num_planes++; + } + for (i = num_planes; i < 4; i++) { if (r->modifier[i]) { DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i); @@ -3242,7 +3249,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret; - if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) { + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS | + DRM_MODE_FB_AUX_PLANE)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL); } @@ -3264,6 +3272,12 @@ internal_framebuffer_create(struct drm_device *dev, return ERR_PTR(-EINVAL); } + if (r->flags & DRM_MODE_FB_AUX_PLANE && + !dev->mode_config.allow_aux_plane) { + DRM_DEBUG_KMS("driver does not support render compression\n"); + return ERR_PTR(-EINVAL); + } + ret = framebuffer_check(r); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 8ce2a0c..ee00782 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_ADDFB2_MODIFIERS: req->value = dev->mode_config.allow_fb_modifiers; break; + case DRM_CAP_RENDER_COMPRESSION: + req->value = dev->mode_config.allow_aux_plane; + break; default: return -EINVAL; } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3f0c690..a5a9da2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1152,6 +1152,9 @@ struct drm_mode_config { /* whether the driver supports fb modifiers */ bool allow_fb_modifiers; + /* whether the driver supports render compression */ + bool allow_aux_plane; + /* cursor size */ uint32_t cursor_width, cursor_height; }; diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 3801584..0834bf7 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -631,6 +631,7 @@ struct drm_gem_open { #define DRM_CAP_CURSOR_WIDTH 0x8 #define DRM_CAP_CURSOR_HEIGHT 0x9 #define DRM_CAP_ADDFB2_MODIFIERS 0x10 +#define DRM_CAP_RENDER_COMPRESSION 0x11 /** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 6c11ca4..de59ace 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd { #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_AUX_PLANE (1<<2) /* for compressed buffer */ struct drm_mode_fb_cmd2 { __u32 fb_id; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/26] drm/i915: move adjusted_mode checks from fbc_update to fbc_enable
Op 29-10-15 om 18:58 schreef Zanoni, Paulo R: > Em Qui, 2015-10-29 às 13:59 +0100, Maarten Lankhorst escreveu: >> Op 27-10-15 om 17:50 schreef Paulo Zanoni: >>> These things can't change without a full modeset. >> False! Fastset can update parameters too. Although I don't think it >> currently prevents DBLSCAN updates, >> so maybe make sure cfb enable/disable is called when updating pipe >> too? > You mean that if we change for an interlaced (or double-scanned) to a > non-interlaced (or non-double-scanned) mode we won't get > crtc_disable+crtc_enable? That's surprising. I'll take a better look > here and try to write a test case. Is this just through some specific > API (like drmModeSetPlane)? > > I'm trying to get test cases for every single bug I discover or > introduce. > Yeah if it does happen it's a bug though. :) ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Allow unready gpu to be reset on gen8
Gen9 has had demonstrated cases where forcing a not ready gpu into reset has caused system hang [1]. Gen8 has never to this date demonstrated such behaviour. In our CI tests there have been two cases of bsw ending in a state where it claims it is not ready for reset, based on reset request, after gpu hang [2]. Both of these cases have happened with kernel where there are lots of debugs enabled. So it is possible that something timing related is at play here like that wait_for_register() usleep wakeups collide badly with forcewake. If we assume that gen8 is safe to reset even with claims of nonreadiness, we can alleviate this situations by forcing a reset and revive the gpu. Enhance logging so that it will be clear what conditions led to decision of proceeding or bailing out, so that we will spot if this way of forcing our will against gpu turns out to be foolhardy. v2: - add bugzilla entry for bsw behaviour (Chris) - improve commit message References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959 References [2]: https://bugs.freedesktop.org/show_bug.cgi?id=92774 Cc: Daniel VetterCc: Tomi Sarvela Cc: Chris Wilson Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_uncore.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index f0f97b2..47c17f2 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1504,7 +1504,14 @@ not_ready: I915_WRITE(RING_RESET_CTL(engine->mmio_base), _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET)); - return -EIO; + if (INTEL_INFO(dev)->gen == 9) { + DRM_ERROR("Reset would risk system stability, bailing out\n"); + return -EIO; + } + + DRM_ERROR("Forcing non ready gpu into reset\n"); + + return gen6_do_reset(dev); } static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 05:39:54AM +, Gong, Zhipeng wrote: > Chris- > > The patch cannot be applied on the latest drm-intel-nightly directly. > I modified it a little bit to make it applied. > The patch can help much in HSW, but a little bit in BDW. > The test is to transcode 26 streams, which creates 244 threads. > > CPU util | w/o patch | w/ patch > -- > HSW async 1 | 102% | 61% > HSW async 5 | 114% | 46% > BDW async 1 | 116% | 116% > BDW async 5 | 111% | 107% The problem on bdw is likely to be frequent inter-ring synchronisation keeping the number of waiters at 1 (i.e. lack of semaphores). Note that the first waiter gets the busywait before waiting on the interrupt. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] skylake + drm-next - warn city
Just booted drm-next on a Skylake laptop that happened to be on my desk for a few days. I wasn't impressed. I'm very disappointed. Doesn't anyone have any pride in the code they write anymore. Initially the previous sentence had a lot of curse words and was Linus like in it's stature, but I've been promised by twitter that being nice will get me better results, so let's make it so. So could someone from Intel takes some responsibility for testing the code they send me actually you know works on the hardware it's meant to, or at least tell me what is going so horribly wrong here. the lockdep trace at the end doesn't look fun. Dave. [8.158254] ACPI: Video Device [GFX0] (multi-head: yes rom: no post: no) [8.159953] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input12 [8.160895] [drm] Initialized i915 1.6.0 20151010 for :00:02.0 on minor 0 [8.170784] [ cut here ] [8.170810] WARNING: CPU: 3 PID: 103 at drivers/gpu/drm/i915/intel_csr.c:481 assert_csr_loaded+0xa8/0x140 [i915]() [8.170812] CSR is not loaded. [8.170813] Modules linked in: amdkfd amd_iommu_v2 amdgpu i915 ttm i2c_algo_bit drm_kms_helper serio_raw drm r8169 mii video fjes [8.170825] CPU: 3 PID: 103 Comm: kworker/u16:2 Not tainted 4.3.0-rc5+ #1 [8.170826] Hardware name: HP HP ProBook 470 G3/8102, BIOS N78 Ver. 01.01 09/04/2015 [8.170830] Workqueue: events_unbound async_run_entry_fn [8.170832] 1aac9e2e 88024bd33a68 81416e09 [8.170835] 88024bd33ab0 88024bd33aa0 810a8bb2 88003f13 [8.170838] 88003f130510 300f 88024ad23000 [8.170841] Call Trace: [8.170845] [] dump_stack+0x4b/0x72 [8.170847] [] warn_slowpath_common+0x82/0xc0 [8.170849] [] warn_slowpath_fmt+0x5c/0x80 [8.170866] [] assert_csr_loaded+0xa8/0x140 [i915] [8.170885] [] skl_set_power_well+0x7e5/0xb00 [i915] [8.170902] [] skl_power_well_enable+0x13/0x20 [i915] [8.170917] [] intel_display_power_get+0xab/0x100 [i915] [8.170944] [] intel_hdmi_set_edid+0x3b/0x110 [i915] [8.170969] [] intel_hdmi_detect+0xc0/0x130 [i915] [8.170974] [] drm_helper_probe_single_connector_modes_merge_bits+0x235/0x4d0 [drm_kms_helper] [8.170978] [] drm_helper_probe_single_connector_modes+0x13/0x20 [drm_kms_helper] [8.170983] [] drm_fb_helper_initial_config+0xb0/0x410 [drm_kms_helper] [8.171007] [] intel_fbdev_initial_config+0x1b/0x20 [i915] [8.171009] [] async_run_entry_fn+0x4a/0x140 [8.171011] [] process_one_work+0x230/0x680 [8.171013] [] ? process_one_work+0x199/0x680 [8.171015] [] worker_thread+0x4e/0x450 [8.171017] [] ? process_one_work+0x680/0x680 [8.171020] [] kthread+0x101/0x120 [8.171023] [] ? trace_hardirqs_on_caller+0x129/0x1b0 [8.171026] [] ? kthread_create_on_node+0x250/0x250 [8.171028] [] ret_from_fork+0x3f/0x70 [8.171031] [] ? kthread_create_on_node+0x250/0x250 [8.171032] ---[ end trace 4692db411b428244 ]--- [8.171035] [ cut here ] [8.171053] WARNING: CPU: 3 PID: 103 at drivers/gpu/drm/i915/intel_csr.c:484 assert_csr_loaded+0x103/0x140 [i915]() [8.171054] CSR SSP Base Not fine [8.171055] Modules linked in: amdkfd amd_iommu_v2 amdgpu i915 ttm i2c_algo_bit drm_kms_helper serio_raw drm r8169 mii video fjes [8.171064] CPU: 3 PID: 103 Comm: kworker/u16:2 Tainted: GW 4.3.0-rc5+ #1 [8.171065] Hardware name: HP HP ProBook 470 G3/8102, BIOS N78 Ver. 01.01 09/04/2015 [8.171067] Workqueue: events_unbound async_run_entry_fn [8.171069] 1aac9e2e 88024bd33a68 81416e09 [8.171071] 88024bd33ab0 88024bd33aa0 810a8bb2 88003f13 [8.171074] 88003f130510 300f 88024ad23000 [8.171077] Call Trace: [8.171079] [] dump_stack+0x4b/0x72 [8.171081] [] warn_slowpath_common+0x82/0xc0 [8.171083] [] warn_slowpath_fmt+0x5c/0x80 [8.171098] [] assert_csr_loaded+0x103/0x140 [i915] [8.171114] [] skl_set_power_well+0x7e5/0xb00 [i915] [8.171129] [] skl_power_well_enable+0x13/0x20 [i915] [8.171143] [] intel_display_power_get+0xab/0x100 [i915] [8.171169] [] intel_hdmi_set_edid+0x3b/0x110 [i915] [8.171191] [] intel_hdmi_detect+0xc0/0x130 [i915] [8.171195] [] drm_helper_probe_single_connector_modes_merge_bits+0x235/0x4d0 [drm_kms_helper] [8.171199] [] drm_helper_probe_single_connector_modes+0x13/0x20 [drm_kms_helper] [8.171204] [] drm_fb_helper_initial_config+0xb0/0x410 [drm_kms_helper] [8.171242] [] intel_fbdev_initial_config+0x1b/0x20 [i915] [8.171244] [] async_run_entry_fn+0x4a/0x140 [8.171247] [] process_one_work+0x230/0x680 [8.171249] [] ? process_one_work+0x199/0x680 [8.171251] [] worker_thread+0x4e/0x450 [8.171253] [] ?
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
On Mon, Nov 02, 2015 at 11:26:29AM +, Gong, Zhipeng wrote: > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: Monday, November 02, 2015 5:59 PM > > To: Gong, Zhipeng > > Cc: intel-gfx@lists.freedesktop.org; Rogozhkin, Dmitry V > > Subject: Re: [PATCH] RFC drm/i915: Slaughter the thundering > > i915_wait_request herd > > > > On Mon, Nov 02, 2015 at 05:39:54AM +, Gong, Zhipeng wrote: > > > Chris- > > > > > > The patch cannot be applied on the latest drm-intel-nightly directly. > > > I modified it a little bit to make it applied. > > > The patch can help much in HSW, but a little bit in BDW. > > > The test is to transcode 26 streams, which creates 244 threads. > > > > > > CPU util | w/o patch | w/ patch > > > -- > > > HSW async 1 | 102% | 61% > > > HSW async 5 | 114% | 46% > > > BDW async 1 | 116% | 116% > > > BDW async 5 | 111% | 107% > > > > Could I get the perf report for the kernel time? One aspect that I find > > hard to > > believe is that it is not the execbuf/mutex-contention that is the > > ratelimiting > > step. > > Sure, what command would you like to run with "perf"? Each of them :) I want to be sure that I know what's going on with bdw (to check if my semaphores guess is correct), and comparing 1-vs-5 should help understand the contention points better. As for the actual command, something like perf report -G -d '[kernel.vmlinux]' | head -5000 should do, though you may have to modify the DSO list to match if you don't use a i915.ko builtin. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
Yeah, very likely. I wonder, how easy is to negotiate issue with inter-ring synchronization on BDW in the expectation of KMD Scheduler from John Harrison? -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, November 2, 2015 12:53 PM To: Gong, Zhipeng Cc: intel-gfx@lists.freedesktop.org; Rogozhkin, Dmitry V Subject: Re: [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd On Mon, Nov 02, 2015 at 05:39:54AM +, Gong, Zhipeng wrote: > Chris- > > The patch cannot be applied on the latest drm-intel-nightly directly. > I modified it a little bit to make it applied. > The patch can help much in HSW, but a little bit in BDW. > The test is to transcode 26 streams, which creates 244 threads. > > CPU util | w/o patch | w/ patch > -- > HSW async 1 | 102% | 61% > HSW async 5 | 114% | 46% > BDW async 1 | 116% | 116% > BDW async 5 | 111% | 107% The problem on bdw is likely to be frequent inter-ring synchronisation keeping the number of waiters at 1 (i.e. lack of semaphores). Note that the first waiter gets the busywait before waiting on the interrupt. -Chris -- Chris Wilson, Intel Open Source Technology Centre Joint Stock Company Intel A/O Registered legal address: Krylatsky Hills Business Park, 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Add extra plane information in debugfs.
Op 27-10-15 om 16:58 schreef Robert Fekete: > Extends i915_display_info so that for each active crtc also print > all planes associated with the pipe. This patch shows information > about each plane wrt format, size, position, rotation, and scaling. > This is very useful when debugging user space compositors that try > to utilize several planes for a commit. > > V2: Fixed comments from Maarten, Ville, and Chris. Fixed printing of > 16.16 fixpoint, better rotation bitmask management and some minor fixes > > V3: Corrected state->src_x & 0x00ff to state->src_x & 0x... > > Signed-off-by: Robert Fekete> Pushed. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] CDCLOCK Sanitization continued for SKL
The cdclock sanitization patch reviewed and merged at - http://patchwork.freedesktop.org/patch/msgid/1445344992-14658-1-git-send-email-shobhit.ku...@intel.com made the assumptions that DPLL should not be enabled when pre-os does not enable display and if it does then verify that the cdclock is corectly programmed as well. The BIOS was actually enabling DPLL as well while not following BSPEC sequence and writing cdclk register directly. I was working with BIOS team to correct this and found that due to a WA needed where audio codec will not be enumerated in OS if BIOS did not program the audio verbs which needed PG2 and DPLL enabling. More discussion revealed the following logic - 1. BIOS puts max cdclk for the platform in CDCLK_CTL. VBIOS/GOP reads that value and then programs cdclk to desired value. 2. It also then sets SWF18 to indicate to the OS that it has enabled display. Used for fastmodeset actually in windows. 3. It also sets SWF06 with this max cdclock(from what bios programmed in CDCLK_CTL) for OS to know. This patch uses point 2 above while sanitizing the cdclk. We can also update our logic for deciding max cdclock based on SWF06 if pre-os enables else directly from CDCLK_CTL (no pre-os display). That is not part of this patch. Shobhit Kumar (1): drm/i915/skl: While sanitizing cdclock check the SWF18 as well drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
> -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Monday, November 02, 2015 5:59 PM > To: Gong, Zhipeng > Cc: intel-gfx@lists.freedesktop.org; Rogozhkin, Dmitry V > Subject: Re: [PATCH] RFC drm/i915: Slaughter the thundering > i915_wait_request herd > > On Mon, Nov 02, 2015 at 05:39:54AM +, Gong, Zhipeng wrote: > > Chris- > > > > The patch cannot be applied on the latest drm-intel-nightly directly. > > I modified it a little bit to make it applied. > > The patch can help much in HSW, but a little bit in BDW. > > The test is to transcode 26 streams, which creates 244 threads. > > > > CPU util | w/o patch | w/ patch > > -- > > HSW async 1 | 102% | 61% > > HSW async 5 | 114% | 46% > > BDW async 1 | 116% | 116% > > BDW async 5 | 111% | 107% > > Could I get the perf report for the kernel time? One aspect that I find hard > to > believe is that it is not the execbuf/mutex-contention that is the > ratelimiting > step. Sure, what command would you like to run with "perf"? -Zhipeng ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: make A0 wa's applied to A1
On Mon, 02 Nov 2015, "Gore, Tim"wrote: >> -Original Message- >> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel >> Vetter >> Sent: Friday, October 30, 2015 4:11 PM >> To: Gore, Tim >> Cc: intel-gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: make A0 wa's applied to A1 >> >> On Mon, Oct 26, 2015 at 10:48:58AM +, tim.g...@intel.com wrote: >> > From: Tim Gore >> > >> > Since A1 chips use the same GPU as A0, they need all the same wa's in >> > the i915 driver. Update some conditionals to do this. >> >> Neither summary nor commit message mentions that this is for bxt. Please >> fix. >> -Daniel >> > This seems to have been merged. Is it possible for me to fix the commit > message > retrospectively (other than in my local repo)? > Tim No. Daniel has a point but it was too late. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Cc: Ville SyrjäläSigned-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i)(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i)(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; + /* +* check if the pre-os intialized the display +* There is SWF18 scratchpad register defined which is set by the +* pre-os which can be used by the OS drivers to check the status +*/ + if ((I915_READ(GEN6_SWF18) & 0x00) == 0) + goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 7/8] lib: add PIPE_ANY to the pipe enum
This avoids compiler warnings about invalid enum values. Signed-off-by: Thomas Wood--- lib/igt_kms.h | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 09c08aa..965c47c 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -40,6 +40,7 @@ /* Low-level helpers with kmstest_ prefix */ enum pipe { +PIPE_ANY = -1, PIPE_A = 0, PIPE_B, PIPE_C, @@ -278,12 +279,6 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe); for (int i__ = 0; (plane) = &(display)->pipes[(pipe)].planes[i__], \ i__ < (display)->pipes[(pipe)].n_planes; i__++) -/* - * Can be used with igt_output_set_pipe() to mean we don't care about the pipe - * that should drive this output - */ -#define PIPE_ANY (-1) - #define IGT_FIXED(i,f) ((i) << 16 | (f)) void igt_enable_connectors(void); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/8] kms_force_connector: use comparison macros to make debug output clearer
Signed-off-by: Thomas Wood--- tests/kms_force_connector.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 7485ca8..4ba1e0b 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -27,8 +27,9 @@ IGT_TEST_DESCRIPTION("Check the debugfs force connector/edid features work" " correctly."); -#define CHECK_MODE(m, h, w, r) igt_assert(m.hdisplay == h && m.vdisplay == w \ - && m.vrefresh == r) +#define CHECK_MODE(m, h, w, r) \ + igt_assert_eq(m.hdisplay, h); igt_assert_eq(m.vdisplay, w); \ + igt_assert_eq(m.vrefresh, r); igt_main { @@ -63,8 +64,8 @@ igt_main /* force the connector on and check the reported values */ kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_ON); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == DRM_MODE_CONNECTED); - igt_assert(temp->count_modes > 0); + igt_assert_eq(temp->connection, DRM_MODE_CONNECTED); + igt_assert_lt(0, temp->count_modes); drmModeFreeConnector(temp); /* attempt to use the display */ @@ -77,15 +78,15 @@ igt_main kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_OFF); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == DRM_MODE_DISCONNECTED); - igt_assert(temp->count_modes == 0); + igt_assert_eq(temp->connection, DRM_MODE_DISCONNECTED); + igt_assert_lt(0, temp->count_modes); drmModeFreeConnector(temp); /* check that the previous state is restored */ kmstest_force_connector(drm_fd, vga_connector, FORCE_CONNECTOR_UNSPECIFIED); temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); - igt_assert(temp->connection == vga_connector->connection); + igt_assert_eq(temp->connection, vga_connector->connection); drmModeFreeConnector(temp); } @@ -115,7 +116,7 @@ igt_main temp = drmModeGetConnector(drm_fd, vga_connector->connector_id); /* the connector should now have the same number of modes that * it started with */ - igt_assert(temp->count_modes == start_n_modes); + igt_assert_eq(temp->count_modes, start_n_modes); drmModeFreeConnector(temp); kmstest_force_connector(drm_fd, vga_connector, -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/8] Add missing noreturn attribute to various functions
Signed-off-by: Thomas Wood--- debugger/eudb.c | 2 +- lib/igt_core.c | 2 +- tests/gem_madvise.c | 2 +- tests/pm_rpm.c | 2 +- tests/testdisplay.c | 2 +- tools/intel_display_poller.c | 2 +- tools/intel_gpu_frequency.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/debugger/eudb.c b/debugger/eudb.c index e015e2e..47d5d92 100644 --- a/debugger/eudb.c +++ b/debugger/eudb.c @@ -326,7 +326,7 @@ db_shutdown(int sig) { printf("Shutting down...\n"); } -static void +static void __attribute__((noreturn)) die(int reason) { int i = 0; diff --git a/lib/igt_core.c b/lib/igt_core.c index 7123455..7e99b24 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -1755,7 +1755,7 @@ out: } static const char *timeout_op; -static void igt_alarm_handler(int signal) +static void __attribute__((noreturn)) igt_alarm_handler(int signal) { if (timeout_op) igt_info("Timed out: %s\n", timeout_op); diff --git a/tests/gem_madvise.c b/tests/gem_madvise.c index 093d78a..36408fe 100644 --- a/tests/gem_madvise.c +++ b/tests/gem_madvise.c @@ -49,7 +49,7 @@ IGT_TEST_DESCRIPTION("Checks that the kernel reports EFAULT when trying to use" static jmp_buf jmp; -static void sigtrap(int sig) +static void __attribute__((noreturn)) sigtrap(int sig) { longjmp(jmp, sig); } diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c index d43cc06..c4fb19c 100644 --- a/tests/pm_rpm.c +++ b/tests/pm_rpm.c @@ -1386,7 +1386,7 @@ static void pci_d3_state_subtest(void) igt_assert(!device_in_pci_d3()); } -static void stay_subtest(void) +static void __attribute__((noreturn)) stay_subtest(void) { disable_all_screens_and_wait(_data); diff --git a/tests/testdisplay.c b/tests/testdisplay.c index 4efcb59..28875b2 100644 --- a/tests/testdisplay.c +++ b/tests/testdisplay.c @@ -552,7 +552,7 @@ static void __attribute__((noreturn)) usage(char *name, char opt) #define dump_resource(res) if (res) dump_##res() -static void cleanup_and_exit(int ret) +static void __attribute__((noreturn)) cleanup_and_exit(int ret) { close(drm_fd); exit(ret); diff --git a/tools/intel_display_poller.c b/tools/intel_display_poller.c index 56dcd44..eab17c5 100644 --- a/tools/intel_display_poller.c +++ b/tools/intel_display_poller.c @@ -946,7 +946,7 @@ static const char *test_name(enum test test, int pipe, int bit, bool test_pixel_ } } -static void usage(const char *name) +static void __attribute__((noreturn)) usage(const char *name) { fprintf(stderr, "Usage: %s [options]\n" " -t,--test \n" diff --git a/tools/intel_gpu_frequency.c b/tools/intel_gpu_frequency.c index 5ae47e7..cb758b0 100644 --- a/tools/intel_gpu_frequency.c +++ b/tools/intel_gpu_frequency.c @@ -142,7 +142,7 @@ static int get_frequency(struct freq_info *freq_info) return val; } -static void +static void __attribute__((noreturn)) usage(const char *prog) { printf("%s A program to manipulate Intel GPU frequencies.\n\n", prog); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 5/8] tests: remove duplicate struct member initializers
Signed-off-by: Thomas Wood--- tests/kms_frontbuffer_tracking.c | 1 - tests/pm_lpsp.c | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c index 15707b9..13b91ad 100644 --- a/tests/kms_frontbuffer_tracking.c +++ b/tests/kms_frontbuffer_tracking.c @@ -305,7 +305,6 @@ drmModeModeInfo std_1024_mode = { .hsync_start = 1048, .hsync_end = 1184, .htotal = 1344, - .vtotal = 806, .hskew = 0, .vdisplay = 768, .vsync_start = 771, diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c index 2badb5c..b62876c 100644 --- a/tests/pm_lpsp.c +++ b/tests/pm_lpsp.c @@ -103,7 +103,6 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res, .hsync_start = 1048, .hsync_end = 1184, .htotal = 1344, - .vtotal = 806, .hskew = 0, .vdisplay = 768, .vsync_start = 771, -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/8] tests: remove unnecessary igt_exit calls
Signed-off-by: Thomas Wood--- tests/gem_render_linear_blits.c | 2 -- tests/gem_render_tiled_blits.c | 2 -- tests/kms_3d.c | 2 -- tests/kms_force_connector.c | 2 -- 4 files changed, 8 deletions(-) diff --git a/tests/gem_render_linear_blits.c b/tests/gem_render_linear_blits.c index 8df180a..13f76a5 100644 --- a/tests/gem_render_linear_blits.c +++ b/tests/gem_render_linear_blits.c @@ -210,6 +210,4 @@ igt_main intel_require_memory(count, SIZE, CHECK_RAM | CHECK_SWAP); run_test(fd, count); } - - igt_exit(); } diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c index 9f6cbd5..fb2f39d 100644 --- a/tests/gem_render_tiled_blits.c +++ b/tests/gem_render_tiled_blits.c @@ -223,6 +223,4 @@ igt_main intel_require_memory(count, SIZE, CHECK_RAM | CHECK_SWAP); run_test(fd, count); } - - igt_exit(); } diff --git a/tests/kms_3d.c b/tests/kms_3d.c index 642a3d6..7484940 100644 --- a/tests/kms_3d.c +++ b/tests/kms_3d.c @@ -115,6 +115,4 @@ igt_simple_main drmModeFreeConnector(connector); free(edid); - - igt_exit(); } diff --git a/tests/kms_force_connector.c b/tests/kms_force_connector.c index 4ba1e0b..6b0cb34 100644 --- a/tests/kms_force_connector.c +++ b/tests/kms_force_connector.c @@ -126,6 +126,4 @@ igt_main igt_fixture { drmModeFreeConnector(vga_connector); } - - igt_exit(); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 8/8] tests/kms_fbc_crc: ensure context is initialized correctly
Initialization was included in commit a976d7e (tests/kms_fbc_crc: refactor context handling code), but won't be executed since it is declared before the first label within a switch statement. kms_fbc_crc.c:178:2: warning: ‘context’ may be used uninitialized in this function [-Wmaybe-uninitialized] rendercopy(batch, context, ^ kms_fbc_crc.c:271:22: note: ‘context’ was declared here drm_intel_context *context = NULL; ^ Cc: Paulo ZanoniSigned-off-by: Thomas Wood --- tests/kms_fbc_crc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c index d580a94..02e95e5 100644 --- a/tests/kms_fbc_crc.c +++ b/tests/kms_fbc_crc.c @@ -255,6 +255,7 @@ static void test_crc(data_t *data, enum test_mode mode) { uint32_t crtc_id = data->output->config.crtc->crtc_id; uint32_t handle = data->fb[0].gem_handle; + drm_intel_context *context = NULL; igt_assert(fbc_enabled(data)); @@ -268,7 +269,6 @@ static void test_crc(data_t *data, enum test_mode mode) } switch (mode) { - drm_intel_context *context = NULL; case TEST_PAGE_FLIP: break; case TEST_MMAP_CPU: -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/8] lib: highlight subtest results on terminals
Make subtest results easier to identify by making them bold when the output is a terminal. Signed-off-by: Thomas Wood--- lib/igt_core.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/igt_core.c b/lib/igt_core.c index 59127ca..7123455 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -779,9 +779,12 @@ bool __igt_run_subtest(const char *subtest_name) } if (skip_subtests_henceforth) { - printf("Subtest %s: %s\n", subtest_name, + bool istty = isatty(STDOUT_FILENO); + + printf("%sSubtest %s: %s%s\n", + (istty) ? "\x1b[1m" : "", subtest_name, skip_subtests_henceforth == SKIP ? - "SKIP" : "FAIL"); + "SKIP" : "FAIL", (istty) ? "\x1b[0m" : ""); return false; } @@ -825,12 +828,14 @@ static void exit_subtest(const char *result) { struct timespec now; double elapsed; + bool istty = isatty(STDOUT_FILENO); gettime(); elapsed = now.tv_sec - subtest_time.tv_sec; elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9; - printf("Subtest %s: %s (%.3fs)\n", in_subtest, result, elapsed); + printf("%sSubtest %s: %s (%.3fs)%s\n", (istty) ? "\x1b[1m" : "", + in_subtest, result, elapsed, (istty) ? "\x1b[0m" : ""); fflush(stdout); in_subtest = NULL; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 6/8] Fix comparison of unsigned integers
Signed-off-by: Thomas Wood--- benchmarks/gem_exec_reloc.c | 2 -- overlay/gem-interrupts.c| 7 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/benchmarks/gem_exec_reloc.c b/benchmarks/gem_exec_reloc.c index 5be482a..2ef6df5 100644 --- a/benchmarks/gem_exec_reloc.c +++ b/benchmarks/gem_exec_reloc.c @@ -249,8 +249,6 @@ int main(int argc, char **argv) case 'r': num_relocs = atoi(optarg); - if (num_relocs < 0) - num_relocs = 0; break; } } diff --git a/overlay/gem-interrupts.c b/overlay/gem-interrupts.c index 48a36b8..0150a1d 100644 --- a/overlay/gem-interrupts.c +++ b/overlay/gem-interrupts.c @@ -142,9 +142,12 @@ int gem_interrupts_update(struct gem_interrupts *irqs) return irqs->error; if (irqs->fd < 0) { - val = interrupts_read(); - if (val < 0) + long long ret; + ret = interrupts_read(); + if (ret < 0) return irqs->error = ENODEV; + else + val = ret; } else { if (read(irqs->fd, , sizeof(val)) < 0) return irqs->error = errno; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] igt/kms_rotation_crc: Add a new subtest to exhaustively test for fence leaks
In this subtest, as a first step, MAX_FENCES+1 number of framebuffers are created backed up by objects that have multiple GGTT views (normal and rotated). Next, we have the i915 driver instantiate a normal view followed by a rotated view. We continue doing the above MAX_FENCES + 1 times. Cc: Tvrtko UrsulinSigned-off-by: Vivek Kasireddy --- tests/kms_rotation_crc.c | 79 1 file changed, 79 insertions(+) diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index ed6eeef..44691d1 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -25,6 +25,7 @@ #include "igt.h" #include +#define MAX_FENCES 32 typedef struct { int gfx_fd; @@ -376,6 +377,78 @@ static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_ty igt_assert(ret == 0); } +static void test_plane_rotation_exhaust_fences(data_t *data, data_t *data2, + enum igt_plane plane_type) +{ + igt_display_t *display = >display; + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + uint32_t format = DRM_FORMAT_XRGB; + int bpp = igt_drm_format_to_bpp(format); + enum igt_commit_style commit = COMMIT_LEGACY; + int fd = data->gfx_fd; + igt_output_t *output = >outputs[0]; + igt_plane_t *plane; + drmModeModeInfo *mode; + unsigned int stride, size, w, h; + uint32_t gem_handle; + int i, ret; + + igt_require(output != NULL && output->valid == true); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + + if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + for (stride = 512; stride < (w * bpp / 8); stride *= 2) + ; + for (size = 1024*1024; size < stride * h; size *= 2) + ; + + igt_plane_set_fb(plane, NULL); + igt_display_commit(display); + + for (i = 0; i < MAX_FENCES + 1; i++) { + gem_handle = gem_create(fd, size); + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride); + igt_assert(ret == 0); + + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, + [i].fb.fb_id)); + data2[i].fb.width = w; + data2[i].fb.height = h; + data2[i].fb.gem_handle = gem_handle; + + igt_plane_set_fb(plane, [i].fb); + igt_plane_set_rotation(plane, IGT_ROTATION_0); + + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + + igt_plane_set_rotation(plane, IGT_ROTATION_90); + + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, +DRM_MODE_OBJECT_PLANE, +plane->rotation_property, +plane->rotation); + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + } + + kmstest_restore_vt_mode(); + + for (i = 0; i < MAX_FENCES + 1; i++) + igt_remove_fb(fd, [i].fb); +} + igt_main { data_t data = {}; @@ -471,6 +544,12 @@ igt_main test_plane_rotation_ytiled_obj(, IGT_PLANE_PRIMARY); } + igt_subtest_f("exhaust-fences") { + data_t data2[MAX_FENCES+1] = {}; + igt_require(gen >= 9); + test_plane_rotation_exhaust_fences(, data2, IGT_PLANE_PRIMARY); + } + igt_fixture { igt_display_fini(); } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Describe the Rotation property bits.
On Mon, Nov 02, 2015 at 03:24:15PM +0100, Robert Fekete wrote: > Adds clarification of the rotation property bits. I.e. rotation is > Counter clockwise and that reflects are applied before any rotation. > > Signed-off-by: Robert Fekete> --- > include/drm/drm_crtc.h | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 3f0c6909dda1..45334fa0cb10 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -85,7 +85,11 @@ static inline uint64_t I642U64(int64_t val) > return (uint64_t)*((uint64_t *)); > } > > -/* rotation property bits */ > +/* > + * Rotation property bits. Rotate-(degrees) the "rotate-" and "reflect-" things were supposed to be the names of the property bits. I guess some quotes around them would have been good. In any case, I tink here we should refer to the define names. eg "DRM_ROTATE_ rotates the image by the ..." > rotates the image by the specified > + * amount in degrees in counter clockwise direction. reflect-x and reflect-y same here with "DRM_REFLECT_X and DRM_REFLECT_Y ..." > + * reflects the image along the specified axis prior to rotation > + */ > #define DRM_ROTATE_MASK 0x0f > #define DRM_ROTATE_0 0 > #define DRM_ROTATE_901 > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: force full detect on sink count change
This patch checks for changes in sink count between short pulse hpds and forces full detect when there is a change. This will allow both detection of hotplug and unplug of panels through dongles that give only short pulse for such events. v2: changed variable type from u8 to bool (Jani) return immediately if perform_full_detect is set(Siva) v3: changed method of determining full detection from using pointer to return code (Siva) Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9a22bec..445ec9c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4461,22 +4461,31 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 * 4. Check link status on receipt of hot-plug interrupt */ -static void +static bool intel_dp_short_pulse(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; u8 sink_irq_vector; u8 link_status[DP_LINK_STATUS_SIZE]; + u8 old_sink_count = intel_dp->sink_count; + bool ret; /* Try to read receiver status if the link appears to be up */ if (!intel_dp_get_link_status(intel_dp, link_status)) { - return; + return false; } - /* Now read the DPCD to see if it's actually running */ - if (!intel_dp_get_dpcd(intel_dp)) { - return; + /* Now read the DPCD to see if it's actually running +* Don't return immediately if dpcd read failed, +* if sink count was 1 and dpcd read failed we need +* to do full detection +*/ + ret = intel_dp_get_dpcd(intel_dp); + + if ((old_sink_count != intel_dp->sink_count) || !ret) { + /* No need to proceed if we are going to do full detect */ + return false; } /* Try to read the source of the interrupt */ @@ -4496,6 +4505,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) drm_modeset_lock(>mode_config.connection_mutex, NULL); intel_dp_check_link_status(intel_dp); drm_modeset_unlock(>mode_config.connection_mutex); + + return true; } /* XXX this is probably wrong for multiple downstream ports */ @@ -5237,7 +5248,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) } if (!intel_dp->is_mst) { - intel_dp_short_pulse(intel_dp); + if (!intel_dp_short_pulse(intel_dp)) { + intel_dp_long_pulse(intel_dp->attached_connector); + goto put_power; + } } } -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Save sink_count for tracking changes to it
Sink count can change between short pulse hpd hence this patch adds a member variable to intel_dp so we can track any changes between short pulse interrupts. Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 7 +++ drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f19945f..e15d42b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4508,14 +4508,13 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) /* If we're HPD-aware, SINK_COUNT changes dynamically */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { - uint8_t reg; if (intel_dp_dpcd_read_wake(_dp->aux, DP_SINK_COUNT, - , 1) < 0) + _dp->sink_count, 1) < 0) return connector_status_unknown; - return DP_GET_SINK_COUNT(reg) ? connector_status_connected - : connector_status_disconnected; + return DP_GET_SINK_COUNT(intel_dp->sink_count) ? + connector_status_connected : connector_status_disconnected; } /* If no HPD, poke DDC gently */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1a3bbdc..abcedc5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -748,6 +748,7 @@ struct intel_dp { uint32_t DP; int link_rate; uint8_t lane_count; + uint8_t sink_count; bool has_audio; enum hdmi_force_audio force_audio; bool limited_color_range; -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: read sink_count dpcd always
This patch reads sink_count dpcd always and removes its read operation based on values in downstream port dpcd. SINK_COUNT dpcd is not dependent on DOWNSTREAM_PORT_PRESENT dpcd. SINK_COUNT denotes if a display is attached, while DOWNSTREAM_PORT_PRESET indicates how many ports are available in the dongle where display can be attached. so it is possible for sink count to change irrespective of value in downstream port dpcd. Here is a table of possible values and scenarios sink_count downstream_port present 0 0 no display is attached 0 1 dongle is connected without display 1 0 display connected directly 1 1 display connected through dongle v2: moved out crtc enabled checks to prior patch(Jani) Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e15d42b..9a22bec 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3988,6 +3988,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] == 0) return false; /* DPCD not present */ + if (intel_dp_dpcd_read_wake(_dp->aux, DP_SINK_COUNT, + _dp->sink_count, 1) < 0) + return false; + + if (!DP_GET_SINK_COUNT(intel_dp->sink_count)) + return false; + /* Check if the panel supports PSR */ memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); if (is_edp(intel_dp)) { @@ -4509,10 +4516,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { - if (intel_dp_dpcd_read_wake(_dp->aux, DP_SINK_COUNT, - _dp->sink_count, 1) < 0) - return connector_status_unknown; - return DP_GET_SINK_COUNT(intel_dp->sink_count) ? connector_status_connected : connector_status_disconnected; } -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect
This patch moves probing for panel, DPCD read etc to another function intel_dp_long_pulse, while intel_dp_detect returns the status as connected or disconnected depending on whether the edid is available or not. This change will be required by further patches in the series to avoid performing multiple DPCD operations and removing their duplication. Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 65 ++--- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1cb1f3f..1677f7c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4785,8 +4785,8 @@ intel_dp_power_put(struct intel_dp *dp, intel_display_power_put(to_i915(encoder->base.dev), power_domain); } -static enum drm_connector_status -intel_dp_detect(struct drm_connector *connector, bool force) +static void +intel_dp_long_pulse(struct drm_connector *connector) { struct intel_dp *intel_dp = intel_attached_dp(connector); struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); @@ -4797,17 +4797,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) bool ret; u8 sink_irq_vector; - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.id, connector->name); - intel_dp_unset_edid(intel_dp); - - if (intel_dp->is_mst) { - /* MST devices are disconnected from a monitor POV */ - if (intel_encoder->type != INTEL_OUTPUT_EDP) - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; - return connector_status_disconnected; - } - power_domain = intel_dp_power_get(intel_dp); /* Can't disconnect eDP, but you can close the lid... */ @@ -4817,19 +4806,35 @@ intel_dp_detect(struct drm_connector *connector, bool force) status = ironlake_dp_detect(intel_dp); else status = g4x_dp_detect(intel_dp); - if (status != connector_status_connected) + if (status != connector_status_connected) { + intel_dp_unset_edid(intel_dp); goto out; + } intel_dp_probe_oui(intel_dp); ret = intel_dp_probe_mst(intel_dp); if (ret) { - /* if we are in MST mode then this connector - won't appear connected or have anything with EDID on it */ + /* +* if we are in MST mode then this connector +* won't appear connected or have anything with +* EDID on it +*/ if (intel_encoder->type != INTEL_OUTPUT_EDP) intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; status = connector_status_disconnected; goto out; + } else if (connector->status == connector_status_connected) { + + /* +* If display was connected already and is still connected +* check links status, there has been known issues of +* link loss triggerring long pulse +*/ + drm_modeset_lock(>mode_config.connection_mutex, NULL); + intel_dp_check_link_status(intel_dp); + drm_modeset_unlock(>mode_config.connection_mutex); + goto out; } intel_dp_set_edid(intel_dp); @@ -4854,7 +4859,33 @@ intel_dp_detect(struct drm_connector *connector, bool force) out: intel_dp_power_put(intel_dp, power_domain); - return status; + return; +} + +static enum drm_connector_status +intel_dp_detect(struct drm_connector *connector, bool force) +{ + struct intel_dp *intel_dp = intel_attached_dp(connector); + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct intel_encoder *intel_encoder = _dig_port->base; + struct intel_connector *intel_connector = to_intel_connector(connector); + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); + + if (intel_dp->is_mst) { + /* MST devices are disconnected from a monitor POV */ + if (intel_encoder->type != INTEL_OUTPUT_EDP) + intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; + return connector_status_disconnected; + } + + intel_dp_long_pulse(intel_dp->attached_connector); + + if (intel_connector->detect_edid) + return connector_status_connected; + else + return connector_status_disconnected; } static void -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Cleaning up intel_dp_hpd_pulse
Current DP detection has DPCD operations split across intel_dp_hpd_pulse and intel_dp_detect which contains duplicates as well. Also intel_dp_detect is called during modes enumeration as well which will result in multiple dpcd operations. So this patch tries to solve both these by bringing all DPCD operations in one single function and make intel_dp_detect use existing values instead of repeating same steps. Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1677f7c..1826a95 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4880,7 +4880,8 @@ intel_dp_detect(struct drm_connector *connector, bool force) return connector_status_disconnected; } - intel_dp_long_pulse(intel_dp->attached_connector); + if (force) + intel_dp_long_pulse(intel_dp->attached_connector); if (intel_connector->detect_edid) return connector_status_connected; @@ -5210,21 +5211,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) /* indicate that we need to restart link training */ intel_dp->train_set_valid = false; - if (!intel_digital_port_connected(dev_priv, intel_dig_port)) - goto mst_fail; + intel_dp_long_pulse(intel_dp->attached_connector); + goto put_power; - if (!intel_dp_get_dpcd(intel_dp)) { - goto mst_fail; - } - - intel_dp_probe_oui(intel_dp); - - if (!intel_dp_probe_mst(intel_dp)) { - drm_modeset_lock(>mode_config.connection_mutex, NULL); - intel_dp_check_link_status(intel_dp); - drm_modeset_unlock(>mode_config.connection_mutex); - goto mst_fail; - } } else { if (intel_dp->is_mst) { if (intel_dp_check_mst_status(intel_dp) == -EINVAL) -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: Splitting intel_dp_check_link_status
The link retraining part when EQ is not correct is retained to intel_dp_check_link_status whereas other operations are handled as part of intel_dp_short_pulse. This change is required to avoid performing all DPCD related operations on performing link retraining. Signed-off-by: Sivakumar ThulasimaniSigned-off-by: Shubhangi Shrivastava --- drivers/gpu/drm/i915/intel_dp.c | 48 + 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1826a95..f19945f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4421,6 +4421,31 @@ go_again: return -EINVAL; } +static void +intel_dp_check_link_status(struct intel_dp *intel_dp) +{ + struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; + u8 link_status[DP_LINK_STATUS_SIZE]; + + if (!intel_dp_get_link_status(intel_dp, link_status)) { + DRM_ERROR("Failed to get link status\n"); + return; + } + + if (!intel_encoder->base.crtc) + return; + + if (!to_intel_crtc(intel_encoder->base.crtc)->active) + return; + + if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) { + DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n", + intel_encoder->base.name); + intel_dp_start_link_train(intel_dp); + intel_dp_stop_link_train(intel_dp); + } +} + /* * According to DP spec * 5.1.2: @@ -4430,21 +4455,13 @@ go_again: * 4. Check link status on receipt of hot-plug interrupt */ static void -intel_dp_check_link_status(struct intel_dp *intel_dp) +intel_dp_short_pulse(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; u8 sink_irq_vector; u8 link_status[DP_LINK_STATUS_SIZE]; - WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex)); - - if (!intel_encoder->base.crtc) - return; - - if (!to_intel_crtc(intel_encoder->base.crtc)->active) - return; - /* Try to read receiver status if the link appears to be up */ if (!intel_dp_get_link_status(intel_dp, link_status)) { return; @@ -4469,12 +4486,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n"); } - if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) { - DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n", - intel_encoder->base.name); - intel_dp_start_link_train(intel_dp); - intel_dp_stop_link_train(intel_dp); - } + drm_modeset_lock(>mode_config.connection_mutex, NULL); + intel_dp_check_link_status(intel_dp); + drm_modeset_unlock(>mode_config.connection_mutex); } /* XXX this is probably wrong for multiple downstream ports */ @@ -5221,9 +5235,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) } if (!intel_dp->is_mst) { - drm_modeset_lock(>mode_config.connection_mutex, NULL); - intel_dp_check_link_status(intel_dp); - drm_modeset_unlock(>mode_config.connection_mutex); + intel_dp_short_pulse(intel_dp); } } -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Fixing sink count related detection over
This patch set cleans up DP detection logic to bring all DPCD operations at one place and to create a clear demarcation between handling of long and short pulses. This simplifies fixing of sink count related detection for DP panels. Patches: 1. First two patches clean up intel_dp_detect and form a new function which will include all DPCD related operations. 2. Second patch splits up intel_dp_check_link_status to form a new function which will handle short pulse requests. 3. Last three patches fixes the detection logic related to sink count i.e detect changes in sink count and handle them appropriately. Note: this is tested on BXT with non-mst panels, will get back ASAP with results for MST panels too. Shubhangi Shrivastava (6): drm/i915: Splitting intel_dp_detect drm/i915: Cleaning up intel_dp_hpd_pulse drm/i915: Splitting intel_dp_check_link_status drm/i915: Save sink_count for tracking changes to it drm/i915: read sink_count dpcd always drm/i915: force full detect on sink count change drivers/gpu/drm/i915/intel_dp.c | 170 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 110 insertions(+), 61 deletions(-) -- 2.6.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Hey, Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com: > From: Ville Syrjälä> > This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9. > > commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully interruptible.") > breaks GPU reset on gen3/4 machines. Go back to to non-interruptible. > I've done some digging and by forcing an unconditional modeset during reset I was able to trigger it on my system. It should be fixed by applying the rest of the interruptible series so we can mask EIO, followed by replacing i915_mutex_lock_interruptible with mutex_lock_interruptible so there will be no waiting for gpu reset. This is what's causing the deadlock here. :) ~Maarten diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 22c0f8a54053..df6dbbc85855 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3207,9 +3207,11 @@ void intel_prepare_reset(struct drm_device *dev) if (IS_GEN2(dev)) return; +#if 0 /* reset doesn't touch the display */ if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) return; +#endif drm_modeset_lock_all(dev); /* @@ -3245,7 +3247,12 @@ void intel_finish_reset(struct drm_device *dev) * FIXME: Atomic will make this obsolete since we won't schedule * CS-based flips (which might get lost in gpu resets) any more. */ +#if 0 intel_update_primary_planes(dev); +#else + intel_display_resume(dev); + drm_modeset_unlock_all(dev); +#endif return; } @@ -13174,12 +13181,12 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, flush_workqueue(dev_priv->wq); } - ret = i915_mutex_lock_interruptible(dev); + ret = mutex_lock_interruptible(>struct_mutex); if (ret) return ret; ret = drm_atomic_helper_prepare_planes(dev, state); - if (!ret && !async) { + if (!ret && !async && !i915_reset_in_progress(_priv->gpu_error)) { u32 reset_counter; reset_counter = atomic_read(_priv->gpu_error.reset_counter); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Add two-stage ILK-style watermark programming (v6)
In addition to calculating final watermarks, let's also pre-calculate a set of intermediate watermark values at atomic check time. These intermediate watermarks are a combination of the watermarks for the old state and the new state; they should satisfy the requirements of both states which means they can be programmed immediately when we commit the atomic state (without waiting for a vblank). Once the vblank does happen, we can then re-program watermarks to the more optimal final value. v2: Significant rebasing/rewriting. v3: - Move 'need_postvbl_update' flag to CRTC state (Daniel) - Don't forget to check intermediate watermark values for validity (Maarten) - Don't due async watermark optimization; just do it at the end of the atomic transaction, after waiting for vblanks. We do want it to be async eventually, but adding that now will cause more trouble for Maarten's in-progress work. (Maarten) - Don't allocate space in crtc_state for intermediate watermarks on platforms that don't need it (gen9+). - Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit now that ilk_update_wm is gone. v4: - Add a wm_mutex to cover updates to intel_crtc->active and the need_postvbl_update flag. Since we don't have async yet it isn't terribly important yet, but might as well add it now. - Change interface to program watermarks. Platforms will now expose .initial_watermarks() and .optimize_watermarks() functions to do watermark programming. These should lock wm_mutex, copy the appropriate state values into intel_crtc->active, and then call the internal program watermarks function. v5: - Skip intermediate watermark calculation/check during initial hardware readout since we don't trust the existing HW values (and don't have valid values of our own yet). - Don't try to call .optimize_watermarks() on platforms that don't have atomic watermarks yet. (Maarten) v6: - Rebase Signed-off-by: Matt RoperReviewed-by(v5): Maarten Lankhorst --- drivers/gpu/drm/i915/i915_drv.h | 6 +- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_display.c | 90 +++- drivers/gpu/drm/i915/intel_drv.h | 31 ++- drivers/gpu/drm/i915/intel_pm.c | 160 --- 5 files changed, 232 insertions(+), 56 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 09807c8..e9a07d8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -629,7 +629,11 @@ struct drm_i915_display_funcs { struct dpll *best_clock); int (*compute_pipe_wm)(struct intel_crtc *crtc, struct drm_atomic_state *state); - void (*program_watermarks)(struct intel_crtc_state *cstate); + int (*compute_intermediate_wm)(struct drm_device *dev, + struct intel_crtc *intel_crtc, + struct intel_crtc_state *newstate); + void (*initial_watermarks)(struct intel_crtc_state *cstate); + void (*optimize_watermarks)(struct intel_crtc_state *cstate); void (*update_wm)(struct drm_crtc *crtc); int (*modeset_calc_cdclk)(struct drm_atomic_state *state); void (*modeset_commit_cdclk)(struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 643f342..b91e166 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -95,6 +95,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->update_pipe = false; crtc_state->disable_lp_wm = false; + crtc_state->wm.need_postvbl_update = false; return _state->base; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 47a67e0..d50884d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11659,6 +11659,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, intel_crtc->atomic.update_wm_pre = true; } + /* Pre-gen9 platforms need two-step watermark updates */ + if ((intel_crtc->atomic.update_wm_pre || intel_crtc->atomic.update_wm_post) && + INTEL_INFO(dev)->gen < 9 && + dev_priv->display.optimize_watermarks) + to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true; + if (visible || was_visible) intel_crtc->atomic.fb_bits |= to_intel_plane(plane)->frontbuffer_bit; @@ -11815,8 +11821,29 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, ret = 0; if (dev_priv->display.compute_pipe_wm) { ret = dev_priv->display.compute_pipe_wm(intel_crtc, state); - if (ret) +
[Intel-gfx] [PATCH 2/4] drm/i915: Add extra paranoia to ILK watermark calculations
Our low-level watermark calculation functions don't get called when the CRTC is disabled or the relevant plane is invisible, so they should never see a zero htotal or zero bpp. However add some checks to ensure this is true so that we don't wind up dividing by zero if we make a mistake elsewhere in the driver (which the atomic watermark series has revealed we might be). References: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077370.html Signed-off-by: Matt Roper--- drivers/gpu/drm/i915/intel_pm.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d9506e2..180348b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1664,6 +1664,9 @@ uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config) if (pipe_h < pfit_h) pipe_h = pfit_h; + if (WARN_ON(!pfit_w || !pfit_h)) + return pixel_rate; + pixel_rate = div_u64((uint64_t) pixel_rate * pipe_w * pipe_h, pfit_w * pfit_h); } @@ -1695,6 +1698,8 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal, if (WARN(latency == 0, "Latency value missing\n")) return UINT_MAX; + if (WARN_ON(!pipe_htotal)) + return UINT_MAX; ret = (latency * pixel_rate) / (pipe_htotal * 1); ret = (ret + 1) * horiz_pixels * bytes_per_pixel; @@ -1705,6 +1710,17 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal, static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels, uint8_t bytes_per_pixel) { + /* +* Neither of these should be possible since this function shouldn't be +* called if the CRTC is off or the plane is invisible. But let's be +* extra paranoid to avoid a potential divide-by-zero if we screw up +* elsewhere in the driver. +*/ + if (WARN_ON(!bytes_per_pixel)) + return 0; + if (WARN_ON(!horiz_pixels)) + return 0; + return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Convert hsw_compute_linetime_wm to use in-flight state
When watermark calculation was moved up to the atomic check phase, the code was updated to calculate based on in-flight atomic state rather than already-committed state. However the hsw_compute_linetime_wm() didn't get updated and continued to pull values out of the currently-committed CRTC state. On platforms that call this function (HSW/BDW only), this will cause problems when we go to enable the CRTC since we'll pull the current mode (off) rather than the mode we're calculating for and wind up with a divide by zero error. This was an oversight in commit: commit a28170f3389f4e42db95e595b0d86384a79de696 Author: Matt RoperDate: Thu Sep 24 15:53:16 2015 -0700 drm/i915: Calculate ILK-style watermarks during atomic check (v3) Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 647c0ff..d9506e2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1990,14 +1990,19 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, } static uint32_t -hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) +hsw_compute_linetime_wm(struct drm_device *dev, + struct intel_crtc_state *cstate) { struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - const struct drm_display_mode *adjusted_mode = _crtc->config->base.adjusted_mode; + const struct drm_display_mode *adjusted_mode = + >base.adjusted_mode; u32 linetime, ips_linetime; - if (!intel_crtc->active) + if (!cstate->base.active) + return 0; + if (WARN_ON(adjusted_mode->crtc_clock == 0)) + return 0; + if (WARN_ON(dev_priv->cdclk_freq == 0)) return 0; /* The WM are computed with base on how long it takes to fill a single @@ -2305,8 +2310,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, pristate, sprstate, curstate, _wm->wm[0]); if (IS_HASWELL(dev) || IS_BROADWELL(dev)) - pipe_wm->linetime = hsw_compute_linetime_wm(dev, - _crtc->base); + pipe_wm->linetime = hsw_compute_linetime_wm(dev, cstate); /* LP0 watermarks always use 1/2 DDB partitioning */ ilk_compute_wm_maximums(dev, 0, , INTEL_DDB_PART_1_2, ); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] Wrap up ILK-style atomic watermarks, try two
Patches #3 and #4 here are the final two patches from the atomic watermark series that was posted here: http://lists.freedesktop.org/archives/intel-gfx/2015-September/076634.html We had to pull those out when Jani reported a BDW boot regression (divide by zero during watermark calculation). Although we never found a smoking gun for that divide by zero, I haven't been able to reproduce the issue on a similar system. There's been a lot of code churn since that time, so I'm hoping that we've either already fixed the issue without realizing it, or that the extra paranoia added in patch #2 here will avoid the crash and highlight the culprit. The first patch here solves a legitimate bug that could cause a divide-by-zero (just not the one Jani was seeing). The second patch adds extra guards on divide operations to verify our invariants and ensure that bugs elsewhere in the driver can't lead to a fatal divide-by-zero (at least on the ILK codepaths). Please don't merge #3 or #4 here until we at least get a positive test result from Jani. Matt Roper (4): drm/i915: Convert hsw_compute_linetime_wm to use in-flight state drm/i915: Add extra paranoia to ILK watermark calculations drm/i915: Sanitize watermarks after hardware state readout drm/i915: Add two-stage ILK-style watermark programming (v6) drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_display.c | 137 +- drivers/gpu/drm/i915/intel_drv.h | 31 +- drivers/gpu/drm/i915/intel_pm.c | 186 +-- 5 files changed, 305 insertions(+), 55 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering i915_wait_request herd
It seems that there are some gaps in the patch and first patch. Like there is no this line in the first patch. if (req->ring->seqno_barrier) I have tried to apply this patch. And here is the cpu utilization and perf data on BDW CPU util | w/o patch | w/ patch -- BDW async 1 | 116% | 95% BDW async 5 | 111% | 91% > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Tuesday, November 03, 2015 4:59 AM > To: Gong, Zhipeng; intel-gfx@lists.freedesktop.org; Rogozhkin, Dmitry V > Subject: Re: [Intel-gfx] [PATCH] RFC drm/i915: Slaughter the thundering > i915_wait_request herd > > On Mon, Nov 02, 2015 at 03:28:22PM +, Chris Wilson wrote: > > That should keep the worker alive for a further 10 jiffies, hopefully > > long enough for the next wait to occur. The cost is that it keeps the > > interrupt asserted (and to avoid that requires a little rearrangment > > and probably a dedicated kthread for each ring). > > Slightly better version that avoids keeping the interrupt active when not > required: > perf.data.tar.gz Description: perf.data.tar.gz ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] CDCLOCK Sanitization continued for SKL
On 11/02/2015 05:25 PM, Shobhit Kumar wrote: The cdclock sanitization patch reviewed and merged at - http://patchwork.freedesktop.org/patch/msgid/1445344992-14658-1-git-send-email-shobhit.ku...@intel.com made the assumptions that DPLL should not be enabled when pre-os does not enable display and if it does then verify that the cdclock is corectly programmed as well. The BIOS was actually enabling DPLL as well while not following BSPEC sequence and writing cdclk register directly. I was working with BIOS team to correct this and found that due to a WA needed where audio codec will not be enumerated in OS if BIOS did not program the audio verbs which needed PG2 and DPLL enabling. More discussion revealed the following logic - 1. BIOS puts max cdclk for the platform in CDCLK_CTL. VBIOS/GOP reads that value and then programs cdclk to desired value. 2. It also then sets SWF18 to indicate to the OS that it has enabled display. Used for fastmodeset actually in windows. 3. It also sets SWF06 with this max cdclock(from what bios programmed in CDCLK_CTL) for OS to know. This patch uses point 2 above while sanitizing the cdclk. We can also update our logic for deciding max cdclock based on SWF06 if pre-os enables else directly from CDCLK_CTL (no pre-os display). That is not part of this patch. One open that comes up now is that with the sanitize implementation, cdclock on SKL is at max 675 MHz when pre-os does not enable display. Given that we do not have dynamic cdclk support yet, this will burn more power in general on lower resolution. Most of the resolutions can be supported on SKL with 337.5 MHz cdclk including 4k@30. IIRC, Ville you were doing something on dynamic cdclk. Is there a plan on supporting that. Regards Shobhit Shobhit Kumar (1): drm/i915/skl: While sanitizing cdclock check the SWF18 as well drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Sanitize watermarks after hardware state readout
Although we can do a good job of reading out hardware state, the graphics firmware may have programmed the watermarks in a creative way that doesn't match how i915 would have chosen to program them. We shouldn't trust the firmware's watermark programming, but should rather re-calculate how we think WM's should be programmed and then shove those values into the hardware. We can do this pretty easily by creating a dummy top-level state, running it through the check process to calculate all the values, and then just programming the watermarks for each CRTC. Signed-off-by: Matt Roper--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 51 drivers/gpu/drm/i915/intel_pm.c | 14 +- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 20cd6d8..09807c8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -629,6 +629,7 @@ struct drm_i915_display_funcs { struct dpll *best_clock); int (*compute_pipe_wm)(struct intel_crtc *crtc, struct drm_atomic_state *state); + void (*program_watermarks)(struct intel_crtc_state *cstate); void (*update_wm)(struct drm_crtc *crtc); int (*modeset_calc_cdclk)(struct drm_atomic_state *state); void (*modeset_commit_cdclk)(struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7b3cfb6..47a67e0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15436,6 +15436,54 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) } } +/* + * Calculate what we think the watermarks should be for the state we've read + * out of the hardware and then immediately program those watermarks so that + * we ensure the hardware settings match our internal state. + */ +static void sanitize_watermarks(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_atomic_state *state; + struct drm_crtc *crtc; + struct drm_crtc_state *cstate; + int ret; + int i; + + /* Only supported on platforms that use atomic watermark design */ + if (!dev_priv->display.program_watermarks) + return; + + /* +* Calculate what we think WM's should be by creating a dummy state and +* running it through the atomic check code. +*/ + state = drm_atomic_helper_duplicate_state(dev, + dev->mode_config.acquire_ctx); + if (WARN_ON(IS_ERR(state))) + return; + + ret = intel_atomic_check(dev, state); + if (ret) { + /* +* Just give up and leave watermarks untouched if we get an +* error back from 'check' +*/ + DRM_DEBUG_KMS("Could not determine valid watermarks for inherited state\n"); + return; + } + + /* Write calculated watermark values back */ + to_i915(dev)->wm.config = to_intel_atomic_state(state)->wm_config; + for_each_crtc_in_state(state, crtc, cstate, i) { + struct intel_crtc_state *cs = to_intel_crtc_state(cstate); + + dev_priv->display.program_watermarks(cs); + } + + drm_atomic_state_free(state); +} + /* Scan out the current hw modeset state, * and sanitizes it to the current state */ @@ -15491,6 +15539,9 @@ intel_modeset_setup_hw_state(struct drm_device *dev) modeset_put_power_domains(dev_priv, put_domains); } intel_display_set_init_power(dev_priv, false); + + /* Make sure hardware watermarks really match the state we read out */ + sanitize_watermarks(dev); } void intel_display_resume(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 180348b..fbcb072 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3611,15 +3611,19 @@ static void skl_update_wm(struct drm_crtc *crtc) dev_priv->wm.skl_hw = *results; } -static void ilk_program_watermarks(struct drm_i915_private *dev_priv) +static void ilk_program_watermarks(struct intel_crtc_state *cstate) { - struct drm_device *dev = dev_priv->dev; + struct drm_crtc *crtc = cstate->base.crtc; + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; struct ilk_wm_maximums max; struct intel_wm_config *config = _priv->wm.config; struct ilk_wm_values results = {}; enum intel_ddb_partitioning partitioning; + to_intel_crtc(crtc)->wm.active.ilk =
Re: [Intel-gfx] [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well
On 11/02/2015 11:48 PM, Thulasimani, Sivakumar wrote: On 11/2/2015 11:17 PM, Kumar, Shobhit wrote: On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote: On 11/2/2015 6:49 PM, Kumar, Shobhit wrote: On 11/02/2015 06:40 PM, Jani Nikula wrote: On Mon, 02 Nov 2015, Shobhit Kumarwrote: SWF18 is set if the display has been intialized by the pre-os. It also gives what configuration is enabled on which pipe. The DPLL and CDCLK verification checks can fail as the pre-os does initialize the DPLL for Audio codec initialization. So fisrt check if SWF18 is set and then follow through with other DPLL and CDCLK verification. Can we universally trust all bios/gop/bootloader/whatnot to have initialized this? What if it's not set? As per my discussion with gop team, this has been enabled in main stream for quite sometime including VLV, CHT, BDW, SKL+ and is common for GOP/VBIOS across chrome/windows/android. So yes I think we can universally trust as of now. This has been added since IVB timeframe and should be part of VBT spec. but i just encountered an issue in Android Charging OS where there is no modeset and is using the displays enabled by GOP/VBIOS. This patch might break such expectations. Why would this break anything in Android charging UI. Basically this patch only says do cdclock sanitization if display is not enabled by pre-os. In this use case it is already enabled by GOP/VBIOS and the driver any way expects it to be programmed by pre-os in general. So in this scenario, the sanitization logic will not do anything at all because SWF18 will be set and CDCLOCK and DPLL will be properly enabled already and just return false. Fast-modeset should not be broken by this at all. my bad :( should review carefully when sitting late night. Regards Shobhit BR, Jani. Cc: Ville Syrjälä Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 8 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ee9481..bd476ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells { #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4) #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4) +/* VBIOS flag for display initialized status */ +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060) + /* Pipe B */ #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000) #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 103cacb..0ecb35c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv) uint32_t cdctl = I915_READ(CDCLK_CTL); int freq = dev_priv->skl_boot_cdclk; +/* + * check if the pre-os intialized the display + * There is SWF18 scratchpad register defined which is set by the + * pre-os which can be used by the OS drivers to check the status + */ +if ((I915_READ(GEN6_SWF18) & 0x00) == 0) +goto sanitize; + /* Is PLL enabled and locked ? */ if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK))) goto sanitize; can you share bit more details on when GOP/VBIOS sets DPLL but does not enable display ? (atleast that is what i understood from the commit message) This patch has to be looked in continuation with the patch for sanitize cdclk which I gave in cover letter to get the full context. Basic point here is that GOP/VBIOS does not enable DPLL as that is not even loaded in certain use cases. It is the FSP/BIOS that loads the GOP and executes it to enable display. In this case, FSP does not load GOP/VBIOS but still programs DPLL itself for some audio codec initialization. The sanitize cdclock function has to decide when to do the sanitization. That was based on DPLL check and CDCLK verification, but both are done by BIOS and hence will assume everything is fine when it is not. That is why additional check of SWF18 is added. I can clarify a bit more in the commit message. Regards Shobhit regards, Sivakumar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx