[Intel-gfx] [PATCH] drm/i915: Also give the sprite width for WM computation
From: Damien Lespiau damien.lesp...@intel.com In the future, we'll need the height of the fb to fetch from memory for WM computation. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_drv.h| 5 - drivers/gpu/drm/i915/intel_pm.c | 17 +++-- drivers/gpu/drm/i915/intel_sprite.c | 15 +-- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3460d801245e..2d0b051fab73 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -440,8 +440,8 @@ struct drm_i915_display_funcs { void (*update_wm)(struct drm_crtc *crtc); void (*update_sprite_wm)(struct drm_plane *plane, struct drm_crtc *crtc, -uint32_t sprite_width, int pixel_size, -bool enable, bool scaled); +uint32_t sprite_width, uint32_t sprite_height, +int pixel_size, bool enable, bool scaled); void (*modeset_global_resources)(struct drm_device *dev); /* Returns the active state of the crtc, and if the crtc is active, * fills out the pipe-config with the hw state. */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f80c6ef659bb..8fd1ea3ac11a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -423,6 +423,7 @@ struct intel_crtc { struct intel_plane_wm_parameters { uint32_t horiz_pixels; + uint32_t vert_pixels; uint8_t bytes_per_pixel; bool enabled; bool scaled; @@ -980,7 +981,9 @@ int ilk_wm_max_level(const struct drm_device *dev); void intel_update_watermarks(struct drm_crtc *crtc); void intel_update_sprite_watermarks(struct drm_plane *plane, struct drm_crtc *crtc, - uint32_t sprite_width, int pixel_size, + uint32_t sprite_width, + uint32_t sprite_height, + int pixel_size, bool enabled, bool scaled); void intel_init_pm(struct drm_device *dev); void intel_pm_setup(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b64b57bbe935..66b723cd5f52 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2743,10 +2743,11 @@ static void ilk_update_wm(struct drm_crtc *crtc) ilk_write_wm_values(dev_priv, results); } -static void ilk_update_sprite_wm(struct drm_plane *plane, -struct drm_crtc *crtc, -uint32_t sprite_width, int pixel_size, -bool enabled, bool scaled) +static void +ilk_update_sprite_wm(struct drm_plane *plane, +struct drm_crtc *crtc, +uint32_t sprite_width, uint32_t sprite_height, +int pixel_size, bool enabled, bool scaled) { struct drm_device *dev = plane-dev; struct intel_plane *intel_plane = to_intel_plane(plane); @@ -2754,6 +2755,7 @@ static void ilk_update_sprite_wm(struct drm_plane *plane, intel_plane-wm.enabled = enabled; intel_plane-wm.scaled = scaled; intel_plane-wm.horiz_pixels = sprite_width; + intel_plane-wm.vert_pixels = sprite_width; intel_plane-wm.bytes_per_pixel = pixel_size; /* @@ -2888,13 +2890,16 @@ void intel_update_watermarks(struct drm_crtc *crtc) void intel_update_sprite_watermarks(struct drm_plane *plane, struct drm_crtc *crtc, - uint32_t sprite_width, int pixel_size, + uint32_t sprite_width, + uint32_t sprite_height, + int pixel_size, bool enabled, bool scaled) { struct drm_i915_private *dev_priv = plane-dev-dev_private; if (dev_priv-display.update_sprite_wm) - dev_priv-display.update_sprite_wm(plane, crtc, sprite_width, + dev_priv-display.update_sprite_wm(plane, crtc, + sprite_width, sprite_height, pixel_size, enabled, scaled); } diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 985317eb1dc9..396c1e843956 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -218,7 +218,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, sprctl |= SP_ENABLE; - intel_update_sprite_watermarks(dplane,
Re: [Intel-gfx] [PATCH] drm/i915: PM irq enabling is generic on gen8, too
On Tue, Jul 15, 2014 at 09:17:41AM +0200, Daniel Vetter wrote: From: Damien Lespiau damien.lesp...@intel.com No need to list all the platforms explicitly. The prefix is a bit inconsistent since we usually pick gen8_ for GT related functions. But this anti-pattern is already established with snb, so material for a different patch. Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Damien Lespiau damien.lesp...@intel.com -- Damien --- 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 c03367af7a8e..e12dfdf4b720 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1403,7 +1403,7 @@ static void gen6_pm_rps_work(struct work_struct *work) spin_lock_irq(dev_priv-irq_lock); pm_iir = dev_priv-rps.pm_iir; dev_priv-rps.pm_iir = 0; - if (IS_BROADWELL(dev_priv-dev) || IS_CHERRYVIEW(dev_priv-dev)) + if (INTEL_INFO(dev_priv-dev)-gen = 8) bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events); else { /* Make sure not to corrupt PMIMR state used by ringbuffer */ -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Add a delay in Displayport AUX transactions for compliance testing
On Mon, Jul 14, 2014 at 12:10:47PM -0700, Todd Previte wrote: The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that repeated AUX transactions after a failure (NACK, DEFER or no response) must have a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically. Signed-off-by: Todd Previte tprev...@gmail.com Since this is a minimal timeout ... shouldn't we put it into the dp helpers instead? -Daniel --- drivers/gpu/drm/i915/intel_dp.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0e207aaf..f0664cd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, DP_AUX_CH_CTL_RECEIVE_ERROR); if (status (DP_AUX_CH_CTL_TIME_OUT_ERROR | - DP_AUX_CH_CTL_RECEIVE_ERROR)) + DP_AUX_CH_CTL_RECEIVE_ERROR)) { + /* DP compliance requires 400us delay for errors/timeouts +(DP CTS 1.2 Core Rev 1.1, 4.2.1.1 4.2.1.2) */ + udelay(400); continue; + } if (status DP_AUX_CH_CTL_DONE) break; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Initialise userptr mmu_notifier serial to 1
On Fri, Jul 11, 2014 at 03:06:55PM +0100, Tvrtko Ursulin wrote: On 07/11/2014 03:02 PM, Daniel Vetter wrote: On Fri, Jul 11, 2014 at 12:06:02PM +0100, Chris Wilson wrote: On Fri, Jul 11, 2014 at 12:00:26PM +0100, Tvrtko Ursulin wrote: On 07/11/2014 11:28 AM, Chris Wilson wrote: During the range invalidate, we walk the list of buffers associated with the mmu_notifer and find the ones that overlap the range. An optimisation is made to speed up the iteration by assuming the previous iter is still valid whilst the tree is unmodified. This exposes a bug when a range invalidate is triggered after we have just created the mmu_notifier, but before attaching any buffers. In that case, we presume we have an unmodified list and start walking from the last iter which is NULL. Oops. The easiest fix is then to initialise the serial of the tree to 1. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursu...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_userptr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 7c38f50014db..8e9e91029aed 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -197,7 +197,7 @@ i915_mmu_notifier_get(struct drm_device *dev, struct mm_struct *mm) mmu-mm = mm; mmu-objects = RB_ROOT; mmu-count = 0; - mmu-serial = 0; + mmu-serial = 1; INIT_LIST_HEAD(mmu-linear); mmu-is_linear = false; Looks good to me and I think safe to merge in any case, so: Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com But it will be interesting to know what code managed to trigger this race, because as we discussed on IRC it would indicate some pretty wild userspace behaviour. Or lack of imagination on our part? A threaded client. One thread using userptr, the other doing munmap or free. Given enough embarrassment, it will happen every time. Can I have an igt please to confirm this and ensure it stays fixed? Sure, give me a day or two. Now that we have the testcase both patches merged, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: PM irq enabling is generic on gen8, too
On Tue, Jul 15, 2014 at 08:26:04AM +0100, Damien Lespiau wrote: On Tue, Jul 15, 2014 at 09:17:41AM +0200, Daniel Vetter wrote: From: Damien Lespiau damien.lesp...@intel.com No need to list all the platforms explicitly. The prefix is a bit inconsistent since we usually pick gen8_ for GT related functions. But this anti-pattern is already established with snb, so material for a different patch. Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Damien Lespiau damien.lesp...@intel.com Both patches merged to dinq. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] lib/igt.cocci: Add s/assert/igt_assert/
People use it way too often, and it upsets the test library. The only valid places to use this is of igt infrastructure self-tests where you need to check something _without_ all the other abi use checks igt_fail and friends do. For those tests just #define an internal assert to hide it. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/igt.cocci | 7 ++ tests/igt_simulation.c | 68 -- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/lib/igt.cocci b/lib/igt.cocci index d399496b256f..adebb31c0277 100644 --- a/lib/igt.cocci +++ b/lib/igt.cocci @@ -84,3 +84,10 @@ expression pipe; + for_each_pipe (display, pipe) { ... } + +// Tests really shouldn't use plain assert! +@@ +expression E; +@@ +- assert(E); ++ igt_assert(E); diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c index a1aa2f46076b..13b2da9ca172 100644 --- a/tests/igt_simulation.c +++ b/tests/igt_simulation.c @@ -34,6 +34,14 @@ #include drmtest.h #include igt_core.h +/* + * We need to hide assert from the cocci igt test refactor spatch. + * + * IMPORTANT: Test infrastructure tests are the only valid places where using + * assert is allowed. + */ +#define internal_assert assert + bool simple; bool list_subtests; bool in_fixture; @@ -50,7 +58,7 @@ static int do_fork(void) switch (pid = fork()) { case -1: - assert(0); + internal_assert(0); case 0: if (simple) { igt_simple_init(1, argv_run); @@ -84,7 +92,7 @@ static int do_fork(void) errno == EINTR) ; - assert(WIFEXITED(status)); + internal_assert(WIFEXITED(status)); return WEXITSTATUS(status); } @@ -94,63 +102,63 @@ int main(int argc, char **argv) { /* simple tests */ simple = true; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SKIP); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SKIP); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); /* subtests, list mode */ simple = false; list_subtests = true; in_fixture = false; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); in_fixture = true; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); in_fixture = false; in_subtest = true; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); /* subtest, run mode */ simple = false; list_subtests = false; in_fixture = false; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SKIP); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SKIP); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); in_fixture = true; - assert(setenv(INTEL_SIMULATION, 1, 1) == 0); - assert(do_fork() == IGT_EXIT_SKIP); + internal_assert(setenv(INTEL_SIMULATION, 1, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SKIP); - assert(setenv(INTEL_SIMULATION, 0, 1) == 0); - assert(do_fork() == IGT_EXIT_SUCCESS); + internal_assert(setenv(INTEL_SIMULATION, 0, 1) == 0); + internal_assert(do_fork() == IGT_EXIT_SUCCESS); in_fixture = false; in_subtest = true; - assert(setenv(INTEL_SIMULATION, 1,
[Intel-gfx] [PATCH 1/2] tests/gem_userptr_blits: Polish
- Drop return values for test functions - we rely on the implicit control flow from igt_ checks. - Don't use assert directly, this upsets the test flow logic (and results in a CRASH result instead of FAIL). Cc: Tvrtko Ursulin tvrtko.ursu...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- tests/gem_userptr_blits.c | 49 ++- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c index f80b4679a747..7efec25bba21 100644 --- a/tests/gem_userptr_blits.c +++ b/tests/gem_userptr_blits.c @@ -42,7 +42,6 @@ #include fcntl.h #include inttypes.h #include errno.h -#include assert.h #include sys/stat.h #include sys/time.h #include sys/mman.h @@ -442,7 +441,7 @@ static int has_userptr(int fd) uint32_t oldflags; int ret; - assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE) == 0); + igt_assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE) == 0); oldflags = userptr_flags; gem_userptr_test_unsynchronized(); ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, handle); @@ -530,9 +529,9 @@ static int test_invalid_mapping(int fd) ptr = gem_mmap__gtt(fd, handle, sizeof(linear), PROT_READ | PROT_WRITE); if (ptr == NULL) gem_close(fd, handle); - assert(ptr != NULL); - assert(((unsigned long)ptr (PAGE_SIZE - 1)) == 0); - assert((sizeof(linear) (PAGE_SIZE - 1)) == 0); + igt_assert(ptr != NULL); + igt_assert(((unsigned long)ptr (PAGE_SIZE - 1)) == 0); + igt_assert((sizeof(linear) (PAGE_SIZE - 1)) == 0); ret = gem_userptr(fd, ptr, sizeof(linear), 0, handle2); igt_assert(ret == 0); copy(fd, handle, handle, ~0); /* QQQ Precise errno? */ @@ -601,7 +600,7 @@ static int test_forbidden_ops(int fd) struct drm_i915_gem_pread gem_pread; struct drm_i915_gem_pwrite gem_pwrite; - assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE) == 0); + igt_assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE) == 0); ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, handle); igt_assert(ret == 0); @@ -716,7 +715,7 @@ static void sigbus(int sig, siginfo_t *info, void *param) if (orig_sigbus) orig_sigbus(sig, info, param); - assert(0); + igt_assert(0); } static int test_dmabuf(void) @@ -763,15 +762,15 @@ static int test_dmabuf(void) sigact.sa_sigaction = sigbus; sigact.sa_flags = SA_SIGINFO; ret = sigaction(SIGBUS, sigact, orig_sigact); - assert(ret == 0); + igt_assert(ret == 0); orig_sigbus = orig_sigact.sa_sigaction; sigbus_cnt = 0; check_bo(fd2, handle_import1, 0, fd2, handle_import1); - assert(sigbus_cnt 0); + igt_assert(sigbus_cnt 0); sigact.sa_sigaction = orig_sigbus; sigact.sa_flags = SA_SIGINFO; ret = sigaction(SIGBUS, sigact, orig_sigact); - assert(ret == 0); + igt_assert(ret == 0); gem_close(fd2, handle_import1); close(fd1); @@ -788,7 +787,7 @@ static int test_usage_restrictions(int fd) int ret; uint32_t handle; - assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE * 2) == 0); + igt_assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE * 2) == 0); /* Address not aligned. */ ret = gem_userptr(fd, (char *)ptr + 1, PAGE_SIZE, 0, handle); @@ -987,7 +986,7 @@ static void test_major_evictions(int fd, int size, int count) major_evictions(fd, fault_ops, size, count); } -static int test_overlap(int fd, int expected) +static void test_overlap(int fd, int expected) { char *ptr; int ret; @@ -1023,11 +1022,9 @@ static int test_overlap(int fd, int expected) gem_close(fd, handle); free(ptr); - - return 0; } -static int test_unmap(int fd, int expected) +static void test_unmap(int fd, int expected) { char *ptr, *bo_ptr; const unsigned int num_obj = 3; @@ -1038,7 +1035,7 @@ static int test_unmap(int fd, int expected) ptr = mmap(NULL, map_size, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); - assert(ptr != MAP_FAILED); + igt_assert(ptr != MAP_FAILED); bo_ptr = (char *)ALIGN((unsigned long)ptr, PAGE_SIZE); @@ -1053,18 +1050,16 @@ static int test_unmap(int fd, int expected) copy(fd, bo[num_obj], bo[i], 0); ret = munmap(ptr, map_size); - assert(ret == 0); + igt_assert(ret == 0); for (i = 0; i num_obj; i++) copy(fd, bo[num_obj], bo[i], expected); for (i = 0; i (num_obj + 1); i++) gem_close(fd, bo[i]); - - return 0; } -static int test_unmap_after_close(int fd) +static void test_unmap_after_close(int fd) { char *ptr, *bo_ptr; const unsigned int num_obj = 3; @@ -1075,7 +1070,7 @@ static int
[Intel-gfx] [PATCH 0/6] Add 180 degree primary and sprite rotation
From: Sonika Jindal sonika.jin...@intel.com This patchset provides support for 0/180 degree hardare rotaion for primary and sprite planes. The rotation property is now made global and is part of drm_mode_config. It is attached to different planes. Sonika Jindal (3): drm: Add rotation_property to mode_config drm/i915: Add 180 degree primary plane rotation support drm: Resetting rotation property Ville Syrjälä (3): drm/i915: Add 180 degree sprite rotation support drm/i915: Make intel_plane_restore() return an error drm/i915: Add rotation property for sprites drivers/gpu/drm/drm_fb_helper.c | 10 +++- drivers/gpu/drm/i915/i915_reg.h |4 ++ drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_drv.h |3 +- drivers/gpu/drm/i915/intel_pm.c |6 ++ drivers/gpu/drm/i915/intel_sprite.c | 93 +++--- include/drm/drm_crtc.h |1 + 7 files changed, 207 insertions(+), 14 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Add 180 degree sprite rotation support
From: Ville Syrjälä ville.syrj...@linux.intel.com The sprite planes (in fact all display planes starting from gen4) support 180 degree rotation. Add the relevant low level bits to the sprite code to make use of that feature. The upper layers are not yet plugged in. v2: HSW handles the rotated buffer offset automagically v3: BDW also handles the rotated buffer offset automagically Testcase: igt/kms_rotation_crc Cc: dri-de...@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |3 +++ drivers/gpu/drm/i915/intel_drv.h|1 + drivers/gpu/drm/i915/intel_sprite.c | 38 +++ 3 files changed, 42 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ce70aa4..74283d5 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4191,6 +4191,7 @@ enum punit_power_well { #define DVS_YUV_ORDER_UYVY (116) #define DVS_YUV_ORDER_YVYU (216) #define DVS_YUV_ORDER_VYUY (316) +#define DVS_ROTATE_180 (115) #define DVS_DEST_KEY (12) #define DVS_TRICKLE_FEED_DISABLE (114) #define DVS_TILED(110) @@ -4261,6 +4262,7 @@ enum punit_power_well { #define SPRITE_YUV_ORDER_UYVY(116) #define SPRITE_YUV_ORDER_YVYU(216) #define SPRITE_YUV_ORDER_VYUY(316) +#define SPRITE_ROTATE_180(115) #define SPRITE_TRICKLE_FEED_DISABLE (114) #define SPRITE_INT_GAMMA_ENABLE (113) #define SPRITE_TILED (110) @@ -4334,6 +4336,7 @@ enum punit_power_well { #define SP_YUV_ORDER_UYVY(116) #define SP_YUV_ORDER_YVYU(216) #define SP_YUV_ORDER_VYUY(316) +#define SP_ROTATE_180(115) #define SP_TILED (110) #define _SPALINOFF (VLV_DISPLAY_BASE + 0x72184) #define _SPASTRIDE (VLV_DISPLAY_BASE + 0x72188) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f80c6ef..88afa44 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -439,6 +439,7 @@ struct intel_plane { unsigned int crtc_w, crtc_h; uint32_t src_x, src_y; uint32_t src_w, src_h; + unsigned int rotation; /* Since we need to change the watermarks before/after * enabling/disabling the planes, we need to store the parameters here diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 985317e..12025fd 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -163,6 +163,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, sprctl = ~SP_PIXFORMAT_MASK; sprctl = ~SP_YUV_BYTE_ORDER_MASK; sprctl = ~SP_TILED; + sprctl = ~SP_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_YUYV: @@ -234,6 +235,14 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, fb-pitches[0]); linear_offset -= sprsurf_offset; + if (intel_plane-rotation == BIT(DRM_ROTATE_180)) { + sprctl |= SP_ROTATE_180; + + x += src_w; + y += src_h; + linear_offset += src_h * fb-pitches[0] + src_w * pixel_size; + } + atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count); intel_update_primary_plane(intel_crtc); @@ -363,6 +372,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, sprctl = ~SPRITE_RGB_ORDER_RGBX; sprctl = ~SPRITE_YUV_BYTE_ORDER_MASK; sprctl = ~SPRITE_TILED; + sprctl = ~SPRITE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_XBGR: @@ -424,6 +434,18 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, pixel_size, fb-pitches[0]); linear_offset -= sprsurf_offset; + if (intel_plane-rotation == BIT(DRM_ROTATE_180)) { + sprctl |= SPRITE_ROTATE_180; + + /* HSW and BDW does this automagically in hardware */ + if (!IS_HASWELL(dev) !IS_BROADWELL(dev)) { + x += src_w; + y += src_h; + linear_offset += src_h * fb-pitches[0] + + src_w * pixel_size; + } + } + atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count); intel_update_primary_plane(intel_crtc); @@ -569,6 +591,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, dvscntr = ~DVS_RGB_ORDER_XBGR; dvscntr = ~DVS_YUV_BYTE_ORDER_MASK; dvscntr =
[Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
From: Sonika Jindal sonika.jin...@intel.com Primary planes support 180 degree rotation. Expose the feature through rotation drm property. v2: Calculating linear/tiled offsets based on pipe source width and height. Added 180 degree rotation support in ironlake_update_plane. v3: Checking if CRTC is active before issueing update_plane. Added wait for vblank to make sure we dont overtake page flips. Disabling FBC since it does not work with rotated planes. v4: Updated rotation checks for pending flips, fbc disable. Creating rotation property only for Gen4 onwards. Property resetting as part of lastclose. v5: Resetting property in i915_driver_lastclose properly for planes and crtcs. Fixed linear offset calculation that was off by 1 w.r.t width in i9xx_update_plane and ironlake_update_plane. Removed tab based indentation and unnecessary braces in intel_crtc_set_property and intel_update_fbc. FBC and flip related checks should be done only for valid crtcs. v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property and positioning the disable code in intel_update_fbc. v7: In case rotation property on inactive crtc is updated, we return successfully printing debug log as crtc is inactive and only property change is preserved. v8: update_plane is changed to update_primary_plane, crtc-fb is changed to crtc-primary-fb and return value of update_primary_plane is ignored. v9: added rotation property to primary plane instead of crtc. Removing reset of rotation property from lastclose. rotation_property is moved to drm_mode_config, so drm layer will take care of resetting. Adding updation of fbc when rotation is set to 0. Allowing rotation only if value is different than old one. Testcase: igt/kms_rotation_crc Signed-off-by: Uma Shankar uma.shan...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 74283d5..f39e2e7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4107,6 +4107,7 @@ enum punit_power_well { #define DISPPLANE_NO_LINE_DOUBLE 0 #define DISPPLANE_STEREO_POLARITY_FIRST 0 #define DISPPLANE_STEREO_POLARITY_SECOND (118) +#define DISPPLANE_ROTATE_180 (115) #define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */ #define DISPPLANE_TILED (110) #define _DSPAADDR 0x70184 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 023c770..e881dcf 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change it */ dspcntr = ~DISPPLANE_PIXFORMAT_MASK; + dspcntr = ~DISPPLANE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_C8: dspcntr |= DISPPLANE_8BPP; @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, if (IS_G4X(dev)) dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; - I915_WRITE(reg, dspcntr); - linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); if (INTEL_INFO(dev)-gen = 4) { @@ -2467,6 +2469,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, } else { intel_crtc-dspaddr_offset = linear_offset; } + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) { + dspcntr |= DISPPLANE_ROTATE_180; + + x += (intel_crtc-config.pipe_src_w - 1); + y += (intel_crtc-config.pipe_src_h - 1); + linear_offset += (intel_crtc-config.pipe_src_h - 1) * + fb-pitches[0] + + (intel_crtc-config.pipe_src_w - 1) * pixel_size; + } + + I915_WRITE(reg, dspcntr); DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n, i915_gem_obj_ggtt_offset(obj), linear_offset, x, y, @@ -2494,11 +2507,14 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change
[Intel-gfx] [PATCH 2/6] drm/i915: Make intel_plane_restore() return an error
From: Ville Syrjälä ville.syrj...@linux.intel.com Propagate the error from intel_update_plane() up through intel_plane_restore() to the caller. This will be used for rollback purposes when setting properties fails. Cc: dri-de...@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_drv.h|2 +- drivers/gpu/drm/i915/intel_sprite.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 88afa44..9bc7fff 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1028,7 +1028,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob); int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane); void intel_flush_primary_plane(struct drm_i915_private *dev_priv, enum plane plane); -void intel_plane_restore(struct drm_plane *plane); +int intel_plane_restore(struct drm_plane *plane); void intel_plane_disable(struct drm_plane *plane); int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 12025fd..9fb7523 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1218,18 +1218,18 @@ out_unlock: return ret; } -void intel_plane_restore(struct drm_plane *plane) +int intel_plane_restore(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); if (!plane-crtc || !plane-fb) - return; + return 0; - intel_update_plane(plane, plane-crtc, plane-fb, - intel_plane-crtc_x, intel_plane-crtc_y, - intel_plane-crtc_w, intel_plane-crtc_h, - intel_plane-src_x, intel_plane-src_y, - intel_plane-src_w, intel_plane-src_h); + return intel_update_plane(plane, plane-crtc, plane-fb, + intel_plane-crtc_x, intel_plane-crtc_y, + intel_plane-crtc_w, intel_plane-crtc_h, + intel_plane-src_x, intel_plane-src_y, + intel_plane-src_w, intel_plane-src_h); } void intel_plane_disable(struct drm_plane *plane) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm: Resetting rotation property
From: Sonika Jindal sonika.jin...@intel.com Reset rotation property to 0 wherever applicable Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3144db9..961afcb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -345,9 +345,17 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) drm_warn_on_modeset_not_all_locked(dev); - list_for_each_entry(plane, dev-mode_config.plane_list, head) + list_for_each_entry(plane, dev-mode_config.plane_list, head) { + + if (dev-mode_config.rotation_property) { + drm_object_property_set_value(plane-base, + dev-mode_config.rotation_property, + BIT(DRM_ROTATE_0)); + } + if (plane-type != DRM_PLANE_TYPE_PRIMARY) drm_plane_force_disable(plane); + } for (i = 0; i fb_helper-crtc_count; i++) { struct drm_mode_set *mode_set = fb_helper-crtc_info[i].mode_set; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm: Add rotation_property to mode_config
From: Sonika Jindal sonika.jin...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- include/drm/drm_crtc.h |1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ce6df4a..5545dd3 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -819,6 +819,7 @@ struct drm_mode_config { struct drm_property *dpms_property; struct drm_property *path_property; struct drm_property *plane_type_property; + struct drm_property *rotation_property; /* DVI-I properties */ struct drm_property *dvi_i_subconnector_property; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Add rotation property for sprites
From: Ville Syrjälä ville.syrj...@linux.intel.com Sprite planes support 180 degree rotation. The lower layers are now in place, so hook in the standard rotation property to expose the feature to the users. v2: Moving rotation_property to mode_config Cc: dri-de...@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 41 ++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9fb7523..94a138d 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1218,6 +1218,30 @@ out_unlock: return ret; } +static int intel_plane_set_property(struct drm_plane *plane, + struct drm_property *prop, + uint64_t val) +{ + struct drm_device *dev = plane-dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + uint64_t old_val; + int ret = -ENOENT; + + if (prop == dev-mode_config.rotation_property) { + /* exactly one rotation angle please */ + if (hweight32(val 0xf) != 1) + return -EINVAL; + + old_val = intel_plane-rotation; + intel_plane-rotation = val; + ret = intel_plane_restore(plane); + if (ret) + intel_plane-rotation = old_val; + } + + return ret; +} + int intel_plane_restore(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); @@ -1244,6 +1268,7 @@ static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, .disable_plane = intel_disable_plane, .destroy = intel_destroy_plane, + .set_property = intel_plane_set_property, }; static uint32_t ilk_plane_formats[] = { @@ -1354,8 +1379,22 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane_funcs, plane_formats, num_plane_formats, false); - if (ret) + if (ret) { kfree(intel_plane); + goto out; + } + + if (!dev-mode_config.rotation_property) + dev-mode_config.rotation_property = + drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | + BIT(DRM_ROTATE_180)); + + if (dev-mode_config.rotation_property) + drm_object_attach_property(intel_plane-base.base, + dev-mode_config.rotation_property, + intel_plane-rotation); + out: return ret; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Primary planes support 180 degree rotation. Expose the feature through rotation drm property. v2: Calculating linear/tiled offsets based on pipe source width and height. Added 180 degree rotation support in ironlake_update_plane. v3: Checking if CRTC is active before issueing update_plane. Added wait for vblank to make sure we dont overtake page flips. Disabling FBC since it does not work with rotated planes. v4: Updated rotation checks for pending flips, fbc disable. Creating rotation property only for Gen4 onwards. Property resetting as part of lastclose. v5: Resetting property in i915_driver_lastclose properly for planes and crtcs. Fixed linear offset calculation that was off by 1 w.r.t width in i9xx_update_plane and ironlake_update_plane. Removed tab based indentation and unnecessary braces in intel_crtc_set_property and intel_update_fbc. FBC and flip related checks should be done only for valid crtcs. v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property and positioning the disable code in intel_update_fbc. v7: In case rotation property on inactive crtc is updated, we return successfully printing debug log as crtc is inactive and only property change is preserved. v8: update_plane is changed to update_primary_plane, crtc-fb is changed to crtc-primary-fb and return value of update_primary_plane is ignored. v9: added rotation property to primary plane instead of crtc. Removing reset of rotation property from lastclose. rotation_property is moved to drm_mode_config, so drm layer will take care of resetting. Adding updation of fbc when rotation is set to 0. Allowing rotation only if value is different than old one. Testcase: igt/kms_rotation_crc Signed-off-by: Uma Shankar uma.shan...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com Some stuff I've noticed below. -Daniel --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 74283d5..f39e2e7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4107,6 +4107,7 @@ enum punit_power_well { #define DISPPLANE_NO_LINE_DOUBLE 0 #define DISPPLANE_STEREO_POLARITY_FIRST0 #define DISPPLANE_STEREO_POLARITY_SECOND (118) +#define DISPPLANE_ROTATE_180 (115) #define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */ #define DISPPLANE_TILED(110) #define _DSPAADDR0x70184 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 023c770..e881dcf 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change it */ dspcntr = ~DISPPLANE_PIXFORMAT_MASK; + dspcntr = ~DISPPLANE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_C8: dspcntr |= DISPPLANE_8BPP; @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, if (IS_G4X(dev)) dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; - I915_WRITE(reg, dspcntr); - linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); if (INTEL_INFO(dev)-gen = 4) { @@ -2467,6 +2469,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, } else { intel_crtc-dspaddr_offset = linear_offset; } + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) { + dspcntr |= DISPPLANE_ROTATE_180; + + x += (intel_crtc-config.pipe_src_w - 1); + y += (intel_crtc-config.pipe_src_h - 1); + linear_offset += (intel_crtc-config.pipe_src_h - 1) * + fb-pitches[0] + + (intel_crtc-config.pipe_src_w - 1) * pixel_size; We already have helper functions to compute the linear offset properly for tiling, I think we should put the rotation adjustments in there to avoid dpulicated code of this. And a comment about how this works would be nice. + } + + I915_WRITE(reg, dspcntr); DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n, i915_gem_obj_ggtt_offset(obj), linear_offset, x,
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
On 7/15/2014 2:41 PM, Daniel Vetter wrote: On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Primary planes support 180 degree rotation. Expose the feature through rotation drm property. v2: Calculating linear/tiled offsets based on pipe source width and height. Added 180 degree rotation support in ironlake_update_plane. v3: Checking if CRTC is active before issueing update_plane. Added wait for vblank to make sure we dont overtake page flips. Disabling FBC since it does not work with rotated planes. v4: Updated rotation checks for pending flips, fbc disable. Creating rotation property only for Gen4 onwards. Property resetting as part of lastclose. v5: Resetting property in i915_driver_lastclose properly for planes and crtcs. Fixed linear offset calculation that was off by 1 w.r.t width in i9xx_update_plane and ironlake_update_plane. Removed tab based indentation and unnecessary braces in intel_crtc_set_property and intel_update_fbc. FBC and flip related checks should be done only for valid crtcs. v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property and positioning the disable code in intel_update_fbc. v7: In case rotation property on inactive crtc is updated, we return successfully printing debug log as crtc is inactive and only property change is preserved. v8: update_plane is changed to update_primary_plane, crtc-fb is changed to crtc-primary-fb and return value of update_primary_plane is ignored. v9: added rotation property to primary plane instead of crtc. Removing reset of rotation property from lastclose. rotation_property is moved to drm_mode_config, so drm layer will take care of resetting. Adding updation of fbc when rotation is set to 0. Allowing rotation only if value is different than old one. Testcase: igt/kms_rotation_crc Signed-off-by: Uma Shankar uma.shan...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com Some stuff I've noticed below. -Daniel --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 74283d5..f39e2e7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4107,6 +4107,7 @@ enum punit_power_well { #define DISPPLANE_NO_LINE_DOUBLE0 #define DISPPLANE_STEREO_POLARITY_FIRST 0 #define DISPPLANE_STEREO_POLARITY_SECOND(118) +#define DISPPLANE_ROTATE_180 (115) #define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */ #define DISPPLANE_TILED (110) #define _DSPAADDR 0x70184 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 023c770..e881dcf 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change it */ dspcntr = ~DISPPLANE_PIXFORMAT_MASK; + dspcntr = ~DISPPLANE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_C8: dspcntr |= DISPPLANE_8BPP; @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, if (IS_G4X(dev)) dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; - I915_WRITE(reg, dspcntr); - linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); if (INTEL_INFO(dev)-gen = 4) { @@ -2467,6 +2469,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, } else { intel_crtc-dspaddr_offset = linear_offset; } + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) { + dspcntr |= DISPPLANE_ROTATE_180; + + x += (intel_crtc-config.pipe_src_w - 1); + y += (intel_crtc-config.pipe_src_h - 1); + linear_offset += (intel_crtc-config.pipe_src_h - 1) * + fb-pitches[0] + + (intel_crtc-config.pipe_src_w - 1) * pixel_size; We already have helper functions to compute the linear offset properly for tiling, I think we should put the rotation adjustments in there to avoid dpulicated code of this. And a comment about how this works would be nice. Ok, I will add the comments. + } + + I915_WRITE(reg, dspcntr); DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n,
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
On Tue, Jul 15, 2014 at 03:08:37PM +0530, Jindal, Sonika wrote: On 7/15/2014 2:41 PM, Daniel Vetter wrote: On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Ok, I will add Also Chris mentioned that on some platforms this won't work and it's more future-proof to just do a full modeset until we have the proper infrastructure. Can you please elaborate on this? What needs to be done? Apparently nothing, it turned out that on the platforms currently supported everything is fine with your patch. Damien promised to follow up with the details on internal mailing lists. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915 modeset question
Adding mailing lists, since I don't know. -Daniel On Mon, Jul 14, 2014 at 9:16 PM, Vladislav Guberinic neosis...@gmail.com wrote: Hi Daniel, I've seen you contributing to i915 driver and writting docs about it, so I thought you might be able to help me. I'm trying to use VGA output of my ivy bridge as a fast DAC. I wrote a little modesetting app and when I hook it up to oscilloscope I get signal. However vsync, and to lesser extent hsync give me problems (in form of harmonics). Is there any way to disable sync signals either in usermode or kernelmode? If you're not the right person to answer this, could you point me in the right direction? Thanks, Vladislav -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/6] Add 180 degree primary and sprite rotation
On Tue, Jul 15, 2014 at 02:10:23PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com This patchset provides support for 0/180 degree hardare rotaion for primary and sprite planes. The rotation property is now made global and is part of drm_mode_config. It is attached to different planes. Sonika Jindal (3): drm: Add rotation_property to mode_config drm/i915: Add 180 degree primary plane rotation support drm: Resetting rotation property Ville Syrjälä (3): drm/i915: Add 180 degree sprite rotation support drm/i915: Make intel_plane_restore() return an error drm/i915: Add rotation property for sprites Also, you forgot to bump the series version, it's really nice to have it to follow the progress (and later, to have tools do that for us). I noticed you likely used --subject-prefix=v3 in the previous one, that's not quite how one is supposed to do it. The series versioning goes into the cover letter subject, and that's it. Cheers, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/6] Add 180 degree primary and sprite rotation
On 7/15/2014 4:22 PM, Damien Lespiau wrote: On Tue, Jul 15, 2014 at 02:10:23PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com This patchset provides support for 0/180 degree hardare rotaion for primary and sprite planes. The rotation property is now made global and is part of drm_mode_config. It is attached to different planes. Sonika Jindal (3): drm: Add rotation_property to mode_config drm/i915: Add 180 degree primary plane rotation support drm: Resetting rotation property Ville Syrjälä (3): drm/i915: Add 180 degree sprite rotation support drm/i915: Make intel_plane_restore() return an error drm/i915: Add rotation property for sprites Also, you forgot to bump the series version, it's really nice to have it to follow the progress (and later, to have tools do that for us). Since this was a new patch set without the previous patches I thought it is not needed. Sure I will keep this in mind for the next time. I noticed you likely used --subject-prefix=v3 in the previous one, that's not quite how one is supposed to do it. The series versioning goes into the cover letter subject, and that's it. I was not aware of this. I thought every patch has to have the version number. Thanks for pointing it out. Cheers, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/6] Add 180 degree primary and sprite rotation
On Tue, Jul 15, 2014 at 04:25:57PM +0530, Jindal, Sonika wrote: Since this was a new patch set without the previous patches I thought it is not needed. Sure I will keep this in mind for the next time. Ah yes, fair enough, my bad. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3 v2] Moving creation of rotation_property to drm layer
From: Sonika Jindal sonika.jin...@intel.com Incorporating comments from Daniel regarding movement of creation of rotation_property to drm_mode_create_standard_plane_properties and other minor things. Sonika Jindal (3): drm: Add rotation_property to mode_config and creating it drm/i915: Add 180 degree primary plane rotation support Ville Syrjälä (1): drm/i915: Add rotation property for sprites drivers/gpu/drm/drm_crtc.c |3 +- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ drivers/gpu/drm/i915/intel_sprite.c | 35 +++- include/drm/drm_crtc.h |1 + 6 files changed, 153 insertions(+), 7 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Add rotation property for sprites
From: Ville Syrjälä ville.syrj...@linux.intel.com Sprite planes support 180 degree rotation. The lower layers are now in place, so hook in the standard rotation property to expose the feature to the users. v2: Moving rotation_property to mode_config v3: Moving creation of property out of intel_sprite Cc: dri-de...@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 35 ++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9fb7523..28c706f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1218,6 +1218,30 @@ out_unlock: return ret; } +static int intel_plane_set_property(struct drm_plane *plane, + struct drm_property *prop, + uint64_t val) +{ + struct drm_device *dev = plane-dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + uint64_t old_val; + int ret = -ENOENT; + + if (prop == dev-mode_config.rotation_property) { + /* exactly one rotation angle please */ + if (hweight32(val 0xf) != 1) + return -EINVAL; + + old_val = intel_plane-rotation; + intel_plane-rotation = val; + ret = intel_plane_restore(plane); + if (ret) + intel_plane-rotation = old_val; + } + + return ret; +} + int intel_plane_restore(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); @@ -1244,6 +1268,7 @@ static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, .disable_plane = intel_disable_plane, .destroy = intel_destroy_plane, + .set_property = intel_plane_set_property, }; static uint32_t ilk_plane_formats[] = { @@ -1354,8 +1379,16 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane_funcs, plane_formats, num_plane_formats, false); - if (ret) + if (ret) { kfree(intel_plane); + goto out; + } + + if (dev-mode_config.rotation_property) + drm_object_attach_property(intel_plane-base.base, + dev-mode_config.rotation_property, + intel_plane-rotation); + out: return ret; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Add 180 degree primary plane rotation support
From: Sonika Jindal sonika.jin...@intel.com Primary planes support 180 degree rotation. Expose the feature through rotation drm property. v2: Calculating linear/tiled offsets based on pipe source width and height. Added 180 degree rotation support in ironlake_update_plane. v3: Checking if CRTC is active before issueing update_plane. Added wait for vblank to make sure we dont overtake page flips. Disabling FBC since it does not work with rotated planes. v4: Updated rotation checks for pending flips, fbc disable. Creating rotation property only for Gen4 onwards. Property resetting as part of lastclose. v5: Resetting property in i915_driver_lastclose properly for planes and crtcs. Fixed linear offset calculation that was off by 1 w.r.t width in i9xx_update_plane and ironlake_update_plane. Removed tab based indentation and unnecessary braces in intel_crtc_set_property and intel_update_fbc. FBC and flip related checks should be done only for valid crtcs. v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property and positioning the disable code in intel_update_fbc. v7: In case rotation property on inactive crtc is updated, we return successfully printing debug log as crtc is inactive and only property change is preserved. v8: update_plane is changed to update_primary_plane, crtc-fb is changed to crtc-primary-fb and return value of update_primary_plane is ignored. v9: added rotation property to primary plane instead of crtc. Removing reset of rotation property from lastclose. rotation_property is moved to drm_mode_config, so drm layer will take care of resetting. Adding updation of fbc when rotation is set to 0. Allowing rotation only if value is different than old one. v10: Moving creation of rotation property out of this file and incorporated other minor comments from Daniel. Testcase: igt/kms_rotation_crc Signed-off-by: Uma Shankar uma.shan...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 74283d5..f39e2e7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4107,6 +4107,7 @@ enum punit_power_well { #define DISPPLANE_NO_LINE_DOUBLE 0 #define DISPPLANE_STEREO_POLARITY_FIRST 0 #define DISPPLANE_STEREO_POLARITY_SECOND (118) +#define DISPPLANE_ROTATE_180 (115) #define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */ #define DISPPLANE_TILED (110) #define _DSPAADDR 0x70184 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 023c770..0d63f90 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change it */ dspcntr = ~DISPPLANE_PIXFORMAT_MASK; + dspcntr = ~DISPPLANE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_C8: dspcntr |= DISPPLANE_8BPP; @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, if (IS_G4X(dev)) dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; - I915_WRITE(reg, dspcntr); - linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); if (INTEL_INFO(dev)-gen = 4) { @@ -2467,6 +2469,20 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, } else { intel_crtc-dspaddr_offset = linear_offset; } + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) { + dspcntr |= DISPPLANE_ROTATE_180; + + x += (intel_crtc-config.pipe_src_w - 1); + y += (intel_crtc-config.pipe_src_h - 1); + + /* Finding the last pixel of the last line of the display data and +* adding to linear_offset*/ + linear_offset += (intel_crtc-config.pipe_src_h - 1) * + fb-pitches[0] + + (intel_crtc-config.pipe_src_w - 1) * pixel_size; + } + + I915_WRITE(reg, dspcntr); DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n, i915_gem_obj_ggtt_offset(obj), linear_offset, x, y, @@ -2494,11 +2510,14 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset;
[Intel-gfx] [PATCH 1/4] drm: Add rotation_property to mode_config and creating it
From: Sonika Jindal sonika.jin...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_crtc.c |3 ++- include/drm/drm_crtc.h |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 787631e..49c0747 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_properties(struct drm_device *dev) type, drm_plane_type_enum_list, ARRAY_SIZE(drm_plane_type_enum_list)); dev-mode_config.plane_type_property = type; - + dev-mode_config.rotation_property = drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); return 0; } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ce6df4a..5545dd3 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -819,6 +819,7 @@ struct drm_mode_config { struct drm_property *dpms_property; struct drm_property *path_property; struct drm_property *plane_type_property; + struct drm_property *rotation_property; /* DVI-I properties */ struct drm_property *dvi_i_subconnector_property; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3 v2] Moving creation of rotation_property to drm layer
On Tue, Jul 15, 2014 at 05:00:20PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Incorporating comments from Daniel regarding movement of creation of rotation_property to drm_mode_create_standard_plane_properties and other minor things. Sonika Jindal (3): drm: Add rotation_property to mode_config and creating it drm/i915: Add 180 degree primary plane rotation support Ville Syrjälä (1): drm/i915: Add rotation property for sprites The summary says 0/3 but the patches are numbered x/4. That's a bit confusing. Two fixes: - Specify the git range for format-patch/send-email with the start..end syntax like for git log. - Send only individual patches in-reply-to the respective original patch. This is my preferred approach. Without that I'll look for 4/4 and won't find it ... Thanks, Daniel drivers/gpu/drm/drm_crtc.c |3 +- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ drivers/gpu/drm/i915/intel_sprite.c | 35 +++- include/drm/drm_crtc.h |1 + 6 files changed, 153 insertions(+), 7 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3 v2] Moving creation of rotation_property to drm layer
On 7/15/2014 5:32 PM, Daniel Vetter wrote: On Tue, Jul 15, 2014 at 05:00:20PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Incorporating comments from Daniel regarding movement of creation of rotation_property to drm_mode_create_standard_plane_properties and other minor things. Sonika Jindal (3): drm: Add rotation_property to mode_config and creating it drm/i915: Add 180 degree primary plane rotation support Ville Syrjälä (1): drm/i915: Add rotation property for sprites The summary says 0/3 but the patches are numbered x/4. That's a bit confusing. Two fixes: - Specify the git range for format-patch/send-email with the start..end syntax like for git log. - Send only individual patches in-reply-to the respective original patch. This is my preferred approach. Ok, I will send each patch in reply to the original patches. I thought replying the affected patches to the comment's mail should make it clearer. I'l will send the patches separately. -Sonika Without that I'll look for 4/4 and won't find it ... Thanks, Daniel drivers/gpu/drm/drm_crtc.c |3 +- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ drivers/gpu/drm/i915/intel_sprite.c | 35 +++- include/drm/drm_crtc.h |1 + 6 files changed, 153 insertions(+), 7 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add 180 degree primary plane rotation support
From: Sonika Jindal sonika.jin...@intel.com Primary planes support 180 degree rotation. Expose the feature through rotation drm property. v2: Calculating linear/tiled offsets based on pipe source width and height. Added 180 degree rotation support in ironlake_update_plane. v3: Checking if CRTC is active before issueing update_plane. Added wait for vblank to make sure we dont overtake page flips. Disabling FBC since it does not work with rotated planes. v4: Updated rotation checks for pending flips, fbc disable. Creating rotation property only for Gen4 onwards. Property resetting as part of lastclose. v5: Resetting property in i915_driver_lastclose properly for planes and crtcs. Fixed linear offset calculation that was off by 1 w.r.t width in i9xx_update_plane and ironlake_update_plane. Removed tab based indentation and unnecessary braces in intel_crtc_set_property and intel_update_fbc. FBC and flip related checks should be done only for valid crtcs. v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property and positioning the disable code in intel_update_fbc. v7: In case rotation property on inactive crtc is updated, we return successfully printing debug log as crtc is inactive and only property change is preserved. v8: update_plane is changed to update_primary_plane, crtc-fb is changed to crtc-primary-fb and return value of update_primary_plane is ignored. v9: added rotation property to primary plane instead of crtc. Removing reset of rotation property from lastclose. rotation_property is moved to drm_mode_config, so drm layer will take care of resetting. Adding updation of fbc when rotation is set to 0. Allowing rotation only if value is different than old one. v10: Moving creation of rotation property out of this file and incorporated other minor comments from Daniel. Testcase: igt/kms_rotation_crc Signed-off-by: Uma Shankar uma.shan...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 104 -- drivers/gpu/drm/i915/intel_pm.c |6 ++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 74283d5..f39e2e7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4107,6 +4107,7 @@ enum punit_power_well { #define DISPPLANE_NO_LINE_DOUBLE 0 #define DISPPLANE_STEREO_POLARITY_FIRST 0 #define DISPPLANE_STEREO_POLARITY_SECOND (118) +#define DISPPLANE_ROTATE_180 (115) #define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */ #define DISPPLANE_TILED (110) #define _DSPAADDR 0x70184 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 023c770..0d63f90 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2408,11 +2408,15 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset; u32 dspcntr; u32 reg; + int pixel_size; + + pixel_size = drm_format_plane_cpp(fb-pixel_format, 0); reg = DSPCNTR(plane); dspcntr = I915_READ(reg); /* Mask out pixel format bits in case we change it */ dspcntr = ~DISPPLANE_PIXFORMAT_MASK; + dspcntr = ~DISPPLANE_ROTATE_180; switch (fb-pixel_format) { case DRM_FORMAT_C8: dspcntr |= DISPPLANE_8BPP; @@ -2454,8 +2458,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, if (IS_G4X(dev)) dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; - I915_WRITE(reg, dspcntr); - linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); if (INTEL_INFO(dev)-gen = 4) { @@ -2467,6 +2469,20 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, } else { intel_crtc-dspaddr_offset = linear_offset; } + if (to_intel_plane(crtc-primary)-rotation == BIT(DRM_ROTATE_180)) { + dspcntr |= DISPPLANE_ROTATE_180; + + x += (intel_crtc-config.pipe_src_w - 1); + y += (intel_crtc-config.pipe_src_h - 1); + + /* Finding the last pixel of the last line of the display data and +* adding to linear_offset*/ + linear_offset += (intel_crtc-config.pipe_src_h - 1) * + fb-pitches[0] + + (intel_crtc-config.pipe_src_w - 1) * pixel_size; + } + + I915_WRITE(reg, dspcntr); DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n, i915_gem_obj_ggtt_offset(obj), linear_offset, x, y, @@ -2494,11 +2510,14 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, unsigned long linear_offset;
[Intel-gfx] [PATCH] drm/i915: Add rotation property for sprites
From: Ville Syrjälä ville.syrj...@linux.intel.com Sprite planes support 180 degree rotation. The lower layers are now in place, so hook in the standard rotation property to expose the feature to the users. v2: Moving rotation_property to mode_config v3: Moving creation of property out of intel_sprite Cc: dri-de...@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 35 ++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9fb7523..28c706f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1218,6 +1218,30 @@ out_unlock: return ret; } +static int intel_plane_set_property(struct drm_plane *plane, + struct drm_property *prop, + uint64_t val) +{ + struct drm_device *dev = plane-dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + uint64_t old_val; + int ret = -ENOENT; + + if (prop == dev-mode_config.rotation_property) { + /* exactly one rotation angle please */ + if (hweight32(val 0xf) != 1) + return -EINVAL; + + old_val = intel_plane-rotation; + intel_plane-rotation = val; + ret = intel_plane_restore(plane); + if (ret) + intel_plane-rotation = old_val; + } + + return ret; +} + int intel_plane_restore(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); @@ -1244,6 +1268,7 @@ static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, .disable_plane = intel_disable_plane, .destroy = intel_destroy_plane, + .set_property = intel_plane_set_property, }; static uint32_t ilk_plane_formats[] = { @@ -1354,8 +1379,16 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane_funcs, plane_formats, num_plane_formats, false); - if (ret) + if (ret) { kfree(intel_plane); + goto out; + } + + if (dev-mode_config.rotation_property) + drm_object_attach_property(intel_plane-base.base, + dev-mode_config.rotation_property, + intel_plane-rotation); + out: return ret; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Add rotation_property to mode_config and creating it
From: Sonika Jindal sonika.jin...@intel.com v2: Adding creation of rotation_property here. Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/drm_crtc.c |3 ++- include/drm/drm_crtc.h |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 787631e..49c0747 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1299,7 +1299,8 @@ static int drm_mode_create_standard_plane_properties(struct drm_device *dev) type, drm_plane_type_enum_list, ARRAY_SIZE(drm_plane_type_enum_list)); dev-mode_config.plane_type_property = type; - + dev-mode_config.rotation_property = drm_mode_create_rotation_property(dev, + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); return 0; } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ce6df4a..5545dd3 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -819,6 +819,7 @@ struct drm_mode_config { struct drm_property *dpms_property; struct drm_property *path_property; struct drm_property *plane_type_property; + struct drm_property *rotation_property; /* DVI-I properties */ struct drm_property *dvi_i_subconnector_property; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder
On 7/14/2014 9:20 PM, Daniel Vetter wrote: On Mon, Jul 14, 2014 at 4:36 PM, Kumar, Shobhit shobhit.ku...@intel.com wrote: /* XXX: read flags, set to adjusted_mode */ + pipe_config-quirks = 1; Nack. First you need to use one of the symbolic quirk definitions (there's a bunch of them). Second this needs a comment why exactly we need the quirk (which really only should be used if there's no way to read a given piece of state back from the hw). Okay, in MIPI we have sync events going as short packets. In that case I think it should be okay to use PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS ? Well it depends. From a quick look it seems like the current dsi code doesn't care at all about sync flags. In that case you should normalize the sync flags of the adjusted mode in the compute_config callback to 0 and not set them in the get_hw_state function. We do that already for e.g. tv encoder outputs. I just assumed that as we don't care about sync flags just suppress their check :) Thanks for pointing correct way of doing this. I will send the corrected patch The quirk flag should only be used if we do set the sync modes but somehow can't read it back. The only case is sdvo where some encoders (in violation of the spec) don't support the flag readback. But that case needs a big comment explaining why. The goal here isn't to shut up the hw cross checker but to actually make it useful ;-) Of-course. Thanks for clarifying. Regards Shobhit ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/8] drm/i915: Add WaDisableFfDopClockGating:bdw
Disable FF DOP clock gating. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 2 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e813033..44e7f34 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5600,6 +5600,7 @@ enum punit_power_well { #define GEN6_RC7 4 #define GEN7_MISCCPCTL (0x9424) +#define GEN8_DOP_CLOCK_GATE_CFCLK_ENABLE (12) #define GEN7_DOP_CLOCK_GATE_ENABLE (10) /* IVYBRIDGE DPF */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8b9abf4..6815680 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5486,6 +5486,10 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* WaProgramL3SqcReg1Default:bdw */ I915_WRITE(GEN8_L3SQCREG1, GEN8_L3SQCREG1_DEFAULT_VALUE); + + /* WaDisableFfDopClockGating:bdw */ + I915_WRITE(GEN7_MISCCPCTL, I915_READ(GEN7_MISCCPCTL) + ~GEN8_DOP_CLOCK_GATE_CFCLK_ENABLE); } static void haswell_init_clock_gating(struct drm_device *dev) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/8] Recent BDW workarounds
These workarounds are mainly for D/E/F steppings. I didn't squash them, because we can eventually get rid of some of them. Michel Thierry (8): drm/i915: Add WaDisableFenceDestinationToSLM:bdw drm/i915: Add WaProgramL3SqcReg1Default:bdw drm/i915: Add WaDisableFfDopClockGating:bdw drm/i915: Add WaFlushCoherentL3CacheLinesAtContextSwitch:bdw drm/i915: Add WaHdcDisableFetchWhenMasked:bdw drm/i915: Add WaIncreaseTagClockTimer:bdw drm/i915: Add WaDisableInstructionShootdown:bdw drm/i915: Add WaDisableLSQCROPERFforOCL:bdw drivers/gpu/drm/i915/i915_reg.h | 16 drivers/gpu/drm/i915/intel_pm.c | 30 +- 2 files changed, 45 insertions(+), 1 deletion(-) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/8] drm/i915: Add WaFlushCoherentL3CacheLinesAtContextSwitch:bdw
Coherent L3 cache lines are not getting flushed during context switch which is causing issues. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 4 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 44e7f34..aaadf4a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4708,6 +4708,9 @@ enum punit_power_well { #define GEN8_L3SQCREG1 0xb100 #define GEN8_L3SQCREG1_DEFAULT_VALUE 0x784000 +#define GEN8_L3SQCREG4 0xb118 +#define GEN8_PIPELINE_FLUSH_COHERENT_LINES(121) + /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 #define GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB (111) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6815680..8bde0aa 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5490,6 +5490,10 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* WaDisableFfDopClockGating:bdw */ I915_WRITE(GEN7_MISCCPCTL, I915_READ(GEN7_MISCCPCTL) ~GEN8_DOP_CLOCK_GATE_CFCLK_ENABLE); + + /* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw */ + I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | + GEN8_PIPELINE_FLUSH_COHERENT_LINES); } static void haswell_init_clock_gating(struct drm_device *dev) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/8] drm/i915: Add WaDisableLSQCROPERFforOCL:bdw
L3SQCREG4 LQSC RO PERF DIS must be programmed by software to 1h (Disable) to work around a Gsync Issue in HDC. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2f0c759..9058ae4 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4714,6 +4714,7 @@ enum punit_power_well { #define GEN8_TAG_CLK_OFFTIME_MASK (~((123) | (122) | (121) | (120))) #define GEN8_L3SQCREG4 0xb118 +#define GEN8_L3SQCREG4_LQSC_RO_PERF_DISABLE (127) #define GEN8_PIPELINE_FLUSH_COHERENT_LINES(121) /* WaCatErrorRejectionIssue */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c16dcb3..41a438e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5495,8 +5495,10 @@ static void gen8_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN7_MISCCPCTL, I915_READ(GEN7_MISCCPCTL) ~GEN8_DOP_CLOCK_GATE_CFCLK_ENABLE); + /* WaDisableLSQCROPERFforOCL:bdw */ /* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw */ I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | + GEN8_L3SQCREG4_LQSC_RO_PERF_DISABLE | GEN8_PIPELINE_FLUSH_COHERENT_LINES); /* -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/8] drm/i915: Add WaDisableInstructionShootdown:bdw
Instruction Shootdown in ROW_CHICKEN must be disabled. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ff1acbe..2f0c759 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5637,6 +5637,7 @@ enum punit_power_well { #define GEN7_PSD_SINGLE_PORT_DISPATCH_ENABLE (13) #define GEN8_ROW_CHICKEN 0xe4f0 +#define INSTRUCTION_SHOOTDOWN_DISABLE(19) #define PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE(18) #define STALL_DOP_GATING_DISABLE (15) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 64e3245..c16dcb3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5405,8 +5405,10 @@ static void gen8_init_clock_gating(struct drm_device *dev) _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ + /* WaDisableInstructionShootdown:bdw */ + /* FIXME: Unclear whether we really need these on production bdw. */ I915_WRITE(GEN8_ROW_CHICKEN, + _MASKED_BIT_ENABLE(INSTRUCTION_SHOOTDOWN_DISABLE) | _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); /* -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] drm/i915: Add WaIncreaseTagClockTimer:bdw
Workaround requires programing L3 tag clock timing register. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 9 + 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 55ea3bf..ff1acbe 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4709,6 +4709,10 @@ enum punit_power_well { #define GEN8_L3SQCREG1 0xb100 #define GEN8_L3SQCREG1_DEFAULT_VALUE 0x784000 +#define GEN8_L3CNTLREG10xb10c +#define GEN8_TAG_CLK_OFFTIME (123) +#define GEN8_TAG_CLK_OFFTIME_MASK (~((123) | (122) | (121) | (120))) + #define GEN8_L3SQCREG4 0xb118 #define GEN8_PIPELINE_FLUSH_COHERENT_LINES(121) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b60fbd2..64e3245 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5496,6 +5496,15 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw */ I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) | GEN8_PIPELINE_FLUSH_COHERENT_LINES); + + /* +* WaIncreaseTagClockTimer:bdw +* This register is not masked and the default value is not all zero. +* Use the mask with and to clear out bits 23:20. +*/ + I915_WRITE(GEN8_L3CNTLREG1, (I915_READ(GEN8_L3CNTLREG1) + GEN8_TAG_CLK_OFFTIME_MASK) | + GEN8_TAG_CLK_OFFTIME); } static void haswell_init_clock_gating(struct drm_device *dev) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] drm/i915: Add WaProgramL3SqcReg1Default:bdw
For performance, program the default initial value of L3SqcReg1 on BDW to 0x784000: L3SQ High Priority Credit Initialization = 2 (1b). L3SQ General Priority Credit Initialization = 30 (0b). Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b8308cf..e813033 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4704,6 +4704,10 @@ enum punit_power_well { #define HDC_FENCE_DESTINATION_TO_SLM_DISABLE (114) #define HDC_FORCE_NON_COHERENT(14) +/* WaProgramL3SqcReg1Default */ +#define GEN8_L3SQCREG1 0xb100 +#define GEN8_L3SQCREG1_DEFAULT_VALUE 0x784000 + /* WaCatErrorRejectionIssue */ #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030 #define GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB (111) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2c3087f..8b9abf4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5483,6 +5483,9 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* Wa4x4STCOptimizationDisable:bdw */ I915_WRITE(CACHE_MODE_1, _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE)); + + /* WaProgramL3SqcReg1Default:bdw */ + I915_WRITE(GEN8_L3SQCREG1, GEN8_L3SQCREG1_DEFAULT_VALUE); } static void haswell_init_clock_gating(struct drm_device *dev) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/8] drm/i915: Add WaHdcDisableFetchWhenMasked:bdw
Set desired default value for HDCCHICKEN register for BDW platform. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index aaadf4a..55ea3bf 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4702,6 +4702,7 @@ enum punit_power_well { /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 #define HDC_FENCE_DESTINATION_TO_SLM_DISABLE (114) +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (111) #define HDC_FORCE_NON_COHERENT(14) /* WaProgramL3SqcReg1Default */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8bde0aa..b60fbd2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5450,10 +5450,12 @@ static void gen8_init_clock_gating(struct drm_device *dev) * workaround for for a possible hang in the unlikely event a TLB * invalidation occurs during a PSD flush. * WaDisableFenceDestinationToSLM:bdw +* WaHdcDisableFetchWhenMasked:bdw */ I915_WRITE(HDC_CHICKEN0, I915_READ(HDC_CHICKEN0) | _MASKED_BIT_ENABLE(HDC_FENCE_DESTINATION_TO_SLM_DISABLE) | + _MASKED_BIT_ENABLE(HDC_DONOT_FETCH_MEM_WHEN_MASKED) | _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT)); /* WaVSRefCountFullforceMissDisable:bdw */ -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/8] drm/i915: Add WaDisableFenceDestinationToSLM:bdw
HDC_CHICKEN0 bit 14 (Fence Destination To SLM Disable) must be programmed by software to 1h (Disable) to work around a LSLM unit issue. WaDisableFenceDestinationToSLM is only needed for BDW E,F step. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ce70aa4..b8308cf 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4701,6 +4701,7 @@ enum punit_power_well { /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 +#define HDC_FENCE_DESTINATION_TO_SLM_DISABLE (114) #define HDC_FORCE_NON_COHERENT(14) /* WaCatErrorRejectionIssue */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6e03851..2c3087f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5449,9 +5449,11 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* Use Force Non-Coherent whenever executing a 3D context. This is a * workaround for for a possible hang in the unlikely event a TLB * invalidation occurs during a PSD flush. +* WaDisableFenceDestinationToSLM:bdw */ I915_WRITE(HDC_CHICKEN0, I915_READ(HDC_CHICKEN0) | + _MASKED_BIT_ENABLE(HDC_FENCE_DESTINATION_TO_SLM_DISABLE) | _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT)); /* WaVSRefCountFullforceMissDisable:bdw */ -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [v2] drm/i915: Add correct hw/sw config check for DSI encoder
Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read. It will not be enabled for DSI and avoid dpio read WARN dumps. Absence of -get_config was causing other WARN dumps as well. Update dpll_hw_state as well correctly v2: Address review comments by Daniel - Check if DPLL is enabled rather than checking pipe output type - set adjusted_mode-flags to 0 in compute_config rather than using pipe_config-quirks - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c - updated dpll_hw_state correctly - Updated commit message and title Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 4 drivers/gpu/drm/i915/intel_dsi.c | 21 +++- drivers/gpu/drm/i915/intel_dsi.h | 1 + drivers/gpu/drm/i915/intel_dsi_pll.c | 46 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c89b4ac..d9c34e4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6132,6 +6132,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, u32 mdiv; int refclk = 10; + /* In case of MIPI DPLL will not even be used */ + if (!(pipe_config-dpll_hw_state.dpll DPLL_VCO_ENABLE)) + return; + mutex_lock(dev_priv-dpio_lock); mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe)); mutex_unlock(dev_priv-dpio_lock); diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index bfcefbf..43be71bf 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder, if (fixed_mode) intel_fixed_panel_mode(fixed_mode, adjusted_mode); + /* DSI uses short packets for sync events, so clear mode flags for DSI */ + adjusted_mode-flags = 0; + if (intel_dsi-dev.dev_ops-mode_fixup) return intel_dsi-dev.dev_ops-mode_fixup(intel_dsi-dev, mode, adjusted_mode); @@ -177,6 +180,10 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) tmp |= DPLL_REFA_CLK_ENABLE_VLV; I915_WRITE(DPLL(pipe), tmp); + /* update the hw state for DPLL */ + intel_crtc-config.dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV | + DPLL_REFA_CLK_ENABLE_VLV; + tmp = I915_READ(DSPCLK_GATE_D); tmp |= DPOUNIT_CLOCK_GATE_DISABLE; I915_WRITE(DSPCLK_GATE_D, tmp); @@ -351,9 +358,21 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder, static void intel_dsi_get_config(struct intel_encoder *encoder, struct intel_crtc_config *pipe_config) { + u32 pclk; DRM_DEBUG_KMS(\n); - /* XXX: read flags, set to adjusted_mode */ + /* +* DPLL_MD is not used in case of DSI, reading will get some default value +* set dpll_md = 0 +*/ + pipe_config-dpll_hw_state.dpll_md = 0; + + pclk = vlv_get_dsi_pclk(encoder, pipe_config-pipe_bpp); + if (!pclk) + return; + + pipe_config-adjusted_mode.crtc_clock = pclk; + pipe_config-port_clock = pclk; } static enum drm_mode_status diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h index 31db33d..fd51867 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder) extern void vlv_enable_dsi_pll(struct intel_encoder *encoder); extern void vlv_disable_dsi_pll(struct intel_encoder *encoder); +extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp); extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops; diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c index ba79ec1..8085afe 100644 --- a/drivers/gpu/drm/i915/intel_dsi_pll.c +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c @@ -50,6 +50,8 @@ static const u32 lfsr_converts[] = { 71, 35 /* 91 - 92 */ }; +static const int num_lfsr_converts = sizeof(lfsr_converts) / sizeof(lfsr_converts[0]); + #ifdef DSI_CLK_FROM_RR static u32 dsi_rr_formula(const struct drm_display_mode *mode, @@ -298,3 +300,47 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder) mutex_unlock(dev_priv-dpio_lock); } + +u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder-base); + u32 dsi_clock, pclk; + u32 pll_ctl, pll_div; + u32 m = 0, p = 0; + int
[Intel-gfx] [PATCH] drm/i915: Make the WRPLL names const
Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index b226724..8f36750 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1200,7 +1200,7 @@ static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, return val WRPLL_PLL_ENABLE; } -static char *hsw_ddi_pll_names[] = { +static const char * const hsw_ddi_pll_names[] = { WRPLL 1, WRPLL 2, }; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make the WRPLL names const
On Tue, Jul 15, 2014 at 03:05:33PM +0100, Damien Lespiau wrote: Signed-off-by: Damien Lespiau damien.lesp...@intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index b226724..8f36750 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1200,7 +1200,7 @@ static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv, return val WRPLL_PLL_ENABLE; } -static char *hsw_ddi_pll_names[] = { +static const char * const hsw_ddi_pll_names[] = { WRPLL 1, WRPLL 2, }; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not access stolen memory directly by the CPU, even for error capture
On Thu, Apr 24, 2014 at 02:47:48PM -0700, Ben Widawsky wrote: On Wed, Feb 12, 2014 at 07:18:40PM +, Chris Wilson wrote: For stolen pages, since it is verboten to access them directly on many architectures, we have to read them through the GTT aperture. If they are not accessible through the aperture, then we have to abort. This was complicated by commit 8b6124a633d8095b0c8364f585edff9c59568a96 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Thu Jan 30 14:38:16 2014 + drm/i915: Don't access snooped pages through the GTT (even for error capture) and the desire to use stolen memory for ringbuffers, contexts and batches in the future. I am somewhat unclear as to whether we want to prefer the aperture for reading back objects which may be mapped in multiple address spaces. Is there a polished version of this floating somewhere which I've missed? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/4] drm/crtc: Add property for aspect ratio
On Jul-15-2014 12:18 PM, Daniel Vetter wrote: [...] I've pulled all 4 patches. Please double-check that I've picked up the right ones since the series is a bit spread out. Thanks, Daniel Hi Daniel, I checked the 4 patches in -next-queued. They are the correct version. Thanks, Vandana ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Add a delay in Displayport AUX transactions for compliance testing
Daniel Vetter mailto:dan...@ffwll.ch Tuesday, July 15, 2014 12:46 AM On Mon, Jul 14, 2014 at 12:10:47PM -0700, Todd Previte wrote: The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that repeated AUX transactions after a failure (NACK, DEFER or no response) must have a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically. Signed-off-by: Todd Previtetprev...@gmail.com Since this is a minimal timeout ... shouldn't we put it into the dp helpers instead? -Daniel This delay catches the case where the sink device either does not respond at all or responds with an invalid AUX transaction. These are lower level errors than the DRM helper functions are supposed to handle. I mistakenly mentioned NACK/DEFER in the commit message - that's not an accurate description of what this patch does. I will fix the commit message for v3 of this patch. -T --- drivers/gpu/drm/i915/intel_dp.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0e207aaf..f0664cd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, DP_AUX_CH_CTL_RECEIVE_ERROR); if (status (DP_AUX_CH_CTL_TIME_OUT_ERROR | - DP_AUX_CH_CTL_RECEIVE_ERROR)) + DP_AUX_CH_CTL_RECEIVE_ERROR)) { + /* DP compliance requires 400us delay for errors/timeouts + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 4.2.1.2) */ + udelay(400); continue; + } if (status DP_AUX_CH_CTL_DONE) break; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Todd Previte mailto:tprev...@gmail.com Monday, July 14, 2014 12:10 PM The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that repeated AUX transactions after a failure (NACK, DEFER or no response) must have a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically. Signed-off-by: Todd Previte tprev...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0e207aaf..f0664cd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, DP_AUX_CH_CTL_RECEIVE_ERROR); if (status (DP_AUX_CH_CTL_TIME_OUT_ERROR | - DP_AUX_CH_CTL_RECEIVE_ERROR)) + DP_AUX_CH_CTL_RECEIVE_ERROR)) { + /* DP compliance requires 400us delay for errors/timeouts + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 4.2.1.2) */ + udelay(400); continue; + } if (status DP_AUX_CH_CTL_DONE) break; } Todd Previte mailto:tprev...@gmail.com Monday, July 14, 2014 12:10 PM V2: - Addressed review feedback from the mailing list - Broke up patches into smaller, easily managed chunks - Reordered the patches such that they can be applied in order - Fixed checkpatch.pl errors across the patchset - Updated and enhanced functionality for the EDID test function - Completely revamped the mode set operations for compliance testing -- Sent with Postbox http://www.getpostbox.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] drm/i915: Minor code clean up in intel_dp.c
Daniel Vetter mailto:dan...@ffwll.ch Tuesday, July 15, 2014 12:47 AM On Mon, Jul 14, 2014 at 12:10:46PM -0700, Todd Previte wrote: Cleans up a couple of unused variables and an extraneous debug log message that was unintentionally left behind. Signed-off-by: Todd Previtetprev...@gmail.com This should be squashed into the relevant earlier patches. -Daniel Sounds good. I'll squash them in and resend the patches as v3. Thanks Daniel. -T --- drivers/gpu/drm/i915/intel_dp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e4b31ad..0e207aaf 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3430,7 +3430,6 @@ intel_dp_autotest_link_training(struct intel_dp *intel_dp) uint8_t rxdata[2]; uint8_t link_status[DP_LINK_STATUS_SIZE]; int bytes_ret = 0; - struct drm_connector *connector =intel_dp-attached_connector-base; struct intel_digital_port *intel_dig_port = enc_to_dig_port(intel_dp-attached_connector-encoder-base); @@ -3494,7 +3493,7 @@ intel_dp_autotest_edid(struct intel_dp *intel_dp) struct edid *edid_read = NULL; uint8_t *edid_data = NULL; uint8_t test_result = DP_TEST_NAK, checksum = 0; - uint32_t i = 0, ret = 0; + uint32_t ret = 0; struct drm_display_mode *use_mode = NULL; int mode_count = 0; struct drm_mode_set modeset; @@ -3592,7 +3591,6 @@ intel_dp_handle_test_request(struct intel_dp *intel_dp) uint8_t rxdata = 0; int ret = 0; - DRM_DEBUG_KMS(Displayport: Received automated test request\n); /* Read DP_TEST_REQUEST register to identify the requested test */ ret = drm_dp_dpcd_read(intel_dp-aux, DP_TEST_REQUEST,rxdata, 1); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Todd Previte mailto:tprev...@gmail.com Monday, July 14, 2014 12:10 PM Cleans up a couple of unused variables and an extraneous debug log message that was unintentionally left behind. Signed-off-by: Todd Previte tprev...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e4b31ad..0e207aaf 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3430,7 +3430,6 @@ intel_dp_autotest_link_training(struct intel_dp *intel_dp) uint8_t rxdata[2]; uint8_t link_status[DP_LINK_STATUS_SIZE]; int bytes_ret = 0; - struct drm_connector *connector = intel_dp-attached_connector-base; struct intel_digital_port *intel_dig_port = enc_to_dig_port(intel_dp-attached_connector-encoder-base); @@ -3494,7 +3493,7 @@ intel_dp_autotest_edid(struct intel_dp *intel_dp) struct edid *edid_read = NULL; uint8_t *edid_data = NULL; uint8_t test_result = DP_TEST_NAK, checksum = 0; - uint32_t i = 0, ret = 0; + uint32_t ret = 0; struct drm_display_mode *use_mode = NULL; int mode_count = 0; struct drm_mode_set modeset; @@ -3592,7 +3591,6 @@ intel_dp_handle_test_request(struct intel_dp *intel_dp) uint8_t rxdata = 0; int ret = 0; - DRM_DEBUG_KMS(Displayport: Received automated test request\n); /* Read DP_TEST_REQUEST register to identify the requested test */ ret = drm_dp_dpcd_read(intel_dp-aux, DP_TEST_REQUEST, rxdata, 1); Todd Previte mailto:tprev...@gmail.com Monday, July 14, 2014 12:10 PM V2: - Addressed review feedback from the mailing list - Broke up patches into smaller, easily managed chunks - Reordered the patches such that they can be applied in order - Fixed checkpatch.pl errors across the patchset - Updated and enhanced functionality for the EDID test function - Completely revamped the mode set operations for compliance testing -- Sent with Postbox http://www.getpostbox.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/14] Enabling GEN8 full PPGTT + fixes v2
This is a rebase from Ben's patches originally sent on July 1st, working in latest drm-nightly. Below is Ben's original cover-letter. == Here be all the patches to make full PPGTT relatively stable on Broadwell. Most of the work was actually to the generic PPGTT code, and not BDW specific. There are basically 3 fixes: 1. Make the error state not horrible, but more work still needed. 2. Fix up another tricky from != from case in do_switch 3. Generally more graceful handling in ppgtt_release 1. Various forms of this subseries have shown up on the list from both myself, and Chris (and I feel like Mika, also). For whatever reason, none have been merged. I don't necessarily mind missing information, but without these patches we can OOPs and GP fault in the error state, which is not acceptable. 2. The meat of the debugging came here. Essentially this problem has already been seen, and solved. The issue is, there is another spot in do_switch that can invoke a switch to the default context. We probably want to get slightly better handling of this, but for now this solves my problems. 3. There are 2 categories addressed in this bucket. Reset, and signals. The patches themselves explain the situation. I take a two step approach with this where first I make things correct (and slow as heck), and then I do the more optimal and trickier solution. Both of these are in line to be replaced by Daniel, but I needed something sooner. Here is the test case that caught all of the above issues: while [ 1 ] ; do (glxgears) pid[0]=$! (glxgears) pid[1]=$! (glxgears) pid[2]=$! sleep 3 kill ${pid[*]} done Ben Widawsky (14): drm/i915: Split up do_switch drm/i915: Extract l3 remapping out of ctx switch drm/i915/ppgtt: Load address space after mi_set_context drm/i915/ctx: Return earlier on failure drm/i915/error: Check the potential ctx obj's vm drm/i915/error: vma error capture prettyify drm/i915/error: Do a better job of disambiguating VMAs drm/i915/error: Capture vmas instead of BOs drm/i915: Add some extra guards in evict_vm drm/i915: Make an uninterruptible evict drm/i915: Reorder ctx unref on ppgtt cleanup drm/i915: More correct (slower) ppgtt cleanup drm/i915: Defer PPGTT cleanup drm/i915/bdw: Enable full PPGTT drivers/gpu/drm/i915/i915_drv.h| 15 +- drivers/gpu/drm/i915/i915_gem.c| 110 ++ drivers/gpu/drm/i915/i915_gem_context.c| 227 ++--- drivers/gpu/drm/i915/i915_gem_evict.c | 39 +++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c| 3 +- drivers/gpu/drm/i915/i915_gem_gtt.h| 4 + drivers/gpu/drm/i915/i915_gpu_error.c | 126 +--- 8 files changed, 411 insertions(+), 115 deletions(-) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/14] drm/i915: Split up do_switch
From: Ben Widawsky benjamin.widaw...@intel.com There are two important reasons for this patch. It should make the existing code a lot more readable. It also makes the next patch much easier to understand in my opinion. There are 2 main variables that effect this function, leaving 4 permutations: ring: RCS vs !RCS PPGTT: full or not I didn't find extracting the full PPGTT usage to be very beneficial at this point, but it may be in the future. This was originally recommended by Daniel Vetter, and in this case, I agree. There was no intentional behavioral change. v2: Change the pin assertion to be GGTT only. This is more accurate. Signed-off-by: Ben Widawsky b...@bwidawsk.net v3: Make it work again after legacy_hw_ctx user_handle changes. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 79 + 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index de72a28..a1dc885 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -611,31 +611,58 @@ mi_set_context(struct intel_engine_cs *ring, return ret; } -static int do_switch(struct intel_engine_cs *ring, -struct intel_context *to) +static void do_switch_fini_common(struct intel_engine_cs *ring, + struct intel_context *from, + struct intel_context *to) +{ + if (likely(from)) + i915_gem_context_unreference(from); + i915_gem_context_reference(to); + ring-last_context = to; +} + +static int do_switch_xcs(struct intel_engine_cs *ring, +struct intel_context *from, +struct intel_context *to) +{ + struct drm_device *dev = ring-dev; + struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to); + int ret; + + BUG_ON(from from-legacy_hw_ctx.rcs_state != NULL); + + if (USES_FULL_PPGTT(dev)) { + ret = ppgtt-switch_mm(ppgtt, ring, false); + if (ret) + return ret; + } + + if (from) + do_switch_fini_common(ring, from, to); + + return 0; +} + +static int do_switch_rcs(struct intel_engine_cs *ring, +struct intel_context *from, +struct intel_context *to) { struct drm_i915_private *dev_priv = ring-dev-dev_private; - struct intel_context *from = ring-last_context; struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to); u32 hw_flags = 0; bool uninitialized = false; int ret, i; - if (from != NULL ring == dev_priv-ring[RCS]) { + if (from != NULL) { BUG_ON(from-legacy_hw_ctx.rcs_state == NULL); - BUG_ON(!i915_gem_obj_is_pinned(from-legacy_hw_ctx.rcs_state)); + BUG_ON(!i915_gem_obj_ggtt_bound(from-legacy_hw_ctx.rcs_state)); } - if (from == to !to-remap_slice) - return 0; - /* Trying to pin first makes error handling easier. */ - if (ring == dev_priv-ring[RCS]) { - ret = i915_gem_obj_ggtt_pin(to-legacy_hw_ctx.rcs_state, - get_context_alignment(ring-dev), 0); - if (ret) - return ret; - } + ret = i915_gem_obj_ggtt_pin(to-legacy_hw_ctx.rcs_state, + get_context_alignment(ring-dev), 0); + if (ret) + return ret; /* * Pin can switch back to the default context if we end up calling into @@ -650,12 +677,6 @@ static int do_switch(struct intel_engine_cs *ring, goto unpin_out; } - if (ring != dev_priv-ring[RCS]) { - if (from) - i915_gem_context_unreference(from); - goto done; - } - /* * Clear this page out of any CPU caches for coherent swap-in/out. Note * that thanks to write = false in this call and us not setting any gpu @@ -714,15 +735,11 @@ static int do_switch(struct intel_engine_cs *ring, /* obj is kept alive until the next request by its active ref */ i915_gem_object_ggtt_unpin(from-legacy_hw_ctx.rcs_state); - i915_gem_context_unreference(from); } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; - -done: - i915_gem_context_reference(to); - ring-last_context = to; + do_switch_fini_common(ring, from, to); if (uninitialized) { ret = i915_gem_render_state_init(ring); @@ -733,8 +750,7 @@ done: return 0; unpin_out: - if (ring-id == RCS) - i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); +
[Intel-gfx] [PATCH 05/14] drm/i915/error: Check the potential ctx obj's vm
From: Ben Widawsky benjamin.widaw...@intel.com The bound list is global (all objects which back the VMAs are stored here). Recently the BUG() in the offset lookup was demoted to a WARN, but the fault actually lies in the caller, here. This bug has existed since the initial introduction of PPGTT (however, it was fixed in unmerged patches to fix up the error state). Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 45b6191..9faebbc 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -920,6 +920,9 @@ static void i915_gem_record_active_context(struct intel_engine_cs *ring, return; list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) { + if (!i915_gem_obj_ggtt_bound(obj)) + continue; + if ((error-ccid PAGE_MASK) == i915_gem_obj_ggtt_offset(obj)) { ering-ctx = i915_error_ggtt_object_create(dev_priv, obj); break; -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/14] drm/i915/ctx: Return earlier on failure
From: Ben Widawsky benjamin.widaw...@intel.com As what was correctly debugged here: commit acc240d41ea1ab9c488a79219fb313b5b46265ae Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Dec 5 15:42:34 2013 +0100 drm/i915: Fix use-after-free in do_switch It then becomes apparent that the default context cannot be the context being switched to for context switch because it is always bound. It follows that if the ring-last_context (from) has changed after the bind_to_gtt, it will always be the default context - this is commented in the code block. This assertion will help catch issues without our logic sooner than letting the system move long (which is possible for some time). I really want this to be a BUG(), but I also want the patch to get merged. I think the fact that none of the ERRNOs make any sense at all is just more evidence that this shouldn't be a WARN. //Cc: Ian Lister (don't have current email address) Cc: Rafael Barbalho rafael.barba...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index cfec178..fab74d3 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -690,6 +690,15 @@ static int do_switch_rcs(struct intel_engine_cs *ring, */ from = ring-last_context; + /* The only context which 'from' can be, if it was changed, is the default +* context. The default context cannot end up in evict everything (as +* commented above) because it is always pinned. +*/ + if (WARN_ON(from == to)) { + ret = -EPERM; + goto unpin_out; + } + if (needs_pd_load) { /* Older GENs still want the load first, PP_DCLV followed by * PP_DIR_BASE register through Load Register Immediate commands -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/14] drm/i915/ppgtt: Load address space after mi_set_context
From: Ben Widawsky benjamin.widaw...@intel.com The simple explanation is, the docs say to do this for GEN8. Perhaps we want to do this for GEN7 too, I am not certain. PDPs are saved and restored with context. Contexts (without execlists) only exist on the render ring. The docs say that PDPs are not power context save/restored. I've learned that this actually means something which SW doesn't care about. So pretend the statement doesn't exist. For non RCS, nothing changes. All this patch now does is change the ordering of LRI vs MI_SET_CONTEXT for the initialization of the context. I do this because the docs say to do it, and frankly, I cannot reason why it is necessary. I've thought about it a lot, and tried, without success, to get a reason from design. The answer I got more or less says, gen7 is different than gen8. I've given up, and am adding this little bit of code to make it in sync with the docs. v2: Completely rewritten commit message that addresses the requests Ville made for v1 Only load PDPs for initial context load (Ville) Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9ab3dad..cfec178 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -669,6 +669,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring, struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to); u32 hw_flags = 0; bool uninitialized = false; + bool needs_pd_load = (INTEL_INFO(ring-dev)-gen 8) USES_FULL_PPGTT(ring-dev); int ret; if (from != NULL) { @@ -689,7 +690,10 @@ static int do_switch_rcs(struct intel_engine_cs *ring, */ from = ring-last_context; - if (USES_FULL_PPGTT(ring-dev)) { + if (needs_pd_load) { + /* Older GENs still want the load first, PP_DCLV followed by +* PP_DIR_BASE register through Load Register Immediate commands +* in Ring Buffer before submitting a context.*/ ret = ppgtt-switch_mm(ppgtt, ring, false); if (ret) goto unpin_out; @@ -713,13 +717,34 @@ static int do_switch_rcs(struct intel_engine_cs *ring, vma-bind_vma(vma, to-legacy_hw_ctx.rcs_state-cache_level, GLOBAL_BIND); } - if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) + if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) { hw_flags |= MI_RESTORE_INHIBIT; + needs_pd_load = USES_FULL_PPGTT(ring-dev) IS_GEN8(ring-dev); + } ret = mi_set_context(ring, to, hw_flags); if (ret) goto unpin_out; + /* GEN8 does *not* require an explicit reload if the PDPs have been +* setup, and we do not wish to move them. +* +* XXX: If we implemented page directory eviction code, this +* optimization needs to be removed. +*/ + if (needs_pd_load) { + ret = ppgtt-switch_mm(ppgtt, ring, false); + /* The hardware context switch is emitted, but we haven't +* actually changed the state - so it's probably safe to bail +* here. Still, let the user know something dangerous has +* happened. +*/ + if (ret) { + DRM_ERROR(Failed to change address space on context switch\n); + goto unpin_out; + } + } + remap_l3(ring, to); /* The backing object for the context is done after switching to the -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/14] drm/i915: Extract l3 remapping out of ctx switch
From: Ben Widawsky benjamin.widaw...@intel.com This is just a cosmetic change to try to put do_switch_rcs on a diet. As it stands, the function was quite complex, and error prone. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a1dc885..9ab3dad 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -643,6 +643,24 @@ static int do_switch_xcs(struct intel_engine_cs *ring, return 0; } +static void remap_l3(struct intel_engine_cs *ring, +struct intel_context *ctx) +{ + int ret, i; + + for (i = 0; i MAX_L3_SLICES; i++) { + if (!(ctx-remap_slice (1i))) + continue; + + ret = i915_gem_l3_remap(ring, i); + /* If it failed, try again next round */ + if (ret) + DRM_DEBUG_DRIVER(L3 remapping failed\n); + else + ctx-remap_slice = ~(1i); + } +} + static int do_switch_rcs(struct intel_engine_cs *ring, struct intel_context *from, struct intel_context *to) @@ -651,7 +669,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring, struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to); u32 hw_flags = 0; bool uninitialized = false; - int ret, i; + int ret; if (from != NULL) { BUG_ON(from-legacy_hw_ctx.rcs_state == NULL); @@ -702,17 +720,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring, if (ret) goto unpin_out; - for (i = 0; i MAX_L3_SLICES; i++) { - if (!(to-remap_slice (1i))) - continue; - - ret = i915_gem_l3_remap(ring, i); - /* If it failed, try again next round */ - if (ret) - DRM_DEBUG_DRIVER(L3 remapping failed\n); - else - to-remap_slice = ~(1i); - } + remap_l3(ring, to); /* The backing object for the context is done after switching to the * *next* context. Therefore we cannot retire the previous context until -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/14] drm/i915/error: vma error capture prettyify
From: Ben Widawsky benjamin.widaw...@intel.com Rename some variables, and clean up the code a bit to make things clearer in our error capture. There isn't an intentional functional change here. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gpu_error.c | 55 --- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 9faebbc..ac101cd 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1016,63 +1016,72 @@ static void i915_gem_record_rings(struct drm_device *dev, } } -/* FIXME: Since pin count/bound list is global, we duplicate what we capture per +/** + * i915_gem_capture_vm() - Capture a VMs error state. + * @error: The main error structure + * @vm:The address space we're capturing. + * @vm_ndx:Which vm without the buffer array + * + * FIXME: Since pin count/bound list is global, we duplicate what we capture per * VM. */ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, struct drm_i915_error_state *error, struct i915_address_space *vm, - const int ndx) + const int vm_ndx) { struct drm_i915_error_buffer *active_bo = NULL, *pinned_bo = NULL; struct drm_i915_gem_object *obj; struct i915_vma *vma; - int i; + int active_vma_count = 0; - i = 0; list_for_each_entry(vma, vm-active_list, mm_list) - i++; - error-active_bo_count[ndx] = i; + active_vma_count++; + + error-active_bo_count[vm_ndx] = active_vma_count; + list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) if (i915_gem_obj_is_pinned(obj)) - i++; - error-pinned_bo_count[ndx] = i - error-active_bo_count[ndx]; + active_vma_count++; + + /* XXX: this is an incorrect measurement of pinned BOs */ + error-pinned_bo_count[vm_ndx] = active_vma_count - error-active_bo_count[vm_ndx]; - if (i) { - active_bo = kcalloc(i, sizeof(*active_bo), GFP_ATOMIC); + if (active_vma_count) { + active_bo = kcalloc(active_vma_count, sizeof(*active_bo), GFP_ATOMIC); if (active_bo) - pinned_bo = active_bo + error-active_bo_count[ndx]; + pinned_bo = active_bo + error-active_bo_count[vm_ndx]; } if (active_bo) - error-active_bo_count[ndx] = + error-active_bo_count[vm_ndx] = capture_active_bo(active_bo, - error-active_bo_count[ndx], + error-active_bo_count[vm_ndx], vm-active_list); if (pinned_bo) - error-pinned_bo_count[ndx] = + error-pinned_bo_count[vm_ndx] = capture_pinned_bo(pinned_bo, - error-pinned_bo_count[ndx], + error-pinned_bo_count[vm_ndx], dev_priv-mm.bound_list); - error-active_bo[ndx] = active_bo; - error-pinned_bo[ndx] = pinned_bo; + error-active_bo[vm_ndx] = active_bo; + error-pinned_bo[vm_ndx] = pinned_bo; } static void i915_gem_capture_buffers(struct drm_i915_private *dev_priv, struct drm_i915_error_state *error) { struct i915_address_space *vm; - int cnt = 0, i = 0; + int vm_count = 0, i = 0; list_for_each_entry(vm, dev_priv-vm_list, global_link) - cnt++; + vm_count++; - error-active_bo = kcalloc(cnt, sizeof(*error-active_bo), GFP_ATOMIC); - error-pinned_bo = kcalloc(cnt, sizeof(*error-pinned_bo), GFP_ATOMIC); - error-active_bo_count = kcalloc(cnt, sizeof(*error-active_bo_count), + error-active_bo = kcalloc(vm_count, sizeof(*error-active_bo), GFP_ATOMIC); + error-pinned_bo = kcalloc(vm_count, sizeof(*error-pinned_bo), GFP_ATOMIC); + error-active_bo_count = kcalloc(vm_count, sizeof(*error-active_bo_count), GFP_ATOMIC); - error-pinned_bo_count = kcalloc(cnt, sizeof(*error-pinned_bo_count), + error-pinned_bo_count = kcalloc(vm_count, sizeof(*error-pinned_bo_count), GFP_ATOMIC); list_for_each_entry(vm, dev_priv-vm_list, global_link) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/14] drm/i915/error: Capture vmas instead of BOs
From: Ben Widawsky benjamin.widaw...@intel.com To follow up on the last patch, we can now capture the VMAs instead of the BOs. The hope if we get more accurate error capture while debugging PPGTT. Note that this does not impact the previous argument about whether to capture all VMAs, or just the guilty VMA. This merely allows the code to do whatever we chose later. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gpu_error.c | 53 +++ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 991b663..96cf5a9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -309,6 +309,7 @@ struct drm_i915_error_state { char error_msg[128]; u32 reset_count; u32 suspend_count; + u32 vm_count; /* Generic register state */ u32 eir; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 4ff819e..eb943ee 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -385,15 +385,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, i915_ring_error_state(m, dev, error-ring[i]); } - if (error-active_bo) - print_error_buffers(m, Active, - error-active_bo[0], - error-active_bo_count[0]); + for (i = 0; i error-vm_count; i++) { + if (error-active_bo[i]) + print_error_buffers(m, Active, + error-active_bo[i], + error-active_bo_count[i]); + } - if (error-pinned_bo) - print_error_buffers(m, Pinned, - error-pinned_bo[0], - error-pinned_bo_count[0]); + for (i = 0; i error-vm_count; i++) { + if (error-pinned_bo[i]) + print_error_buffers(m, Pinned, + error-pinned_bo[i], + error-pinned_bo_count[i]); + } for (i = 0; i ARRAY_SIZE(error-ring); i++) { obj = error-ring[i].batchbuffer; @@ -635,22 +639,23 @@ unwind: i915_error_object_create_sized((dev_priv), (src), (dev_priv)-gtt.base, \ (src)-base.sizePAGE_SHIFT) -static void capture_bo(struct drm_i915_error_buffer *err, - struct drm_i915_gem_object *obj) +static void capture_vma(struct drm_i915_error_buffer *err, struct i915_vma *vma) { + struct drm_i915_gem_object *obj = vma-obj; + err-size = obj-base.size; err-name = obj-base.name; err-rseqno = obj-last_read_seqno; err-wseqno = obj-last_write_seqno; - err-gtt_offset = i915_gem_obj_ggtt_offset(obj); + err-gtt_offset = vma-node.start; err-read_domains = obj-base.read_domains; err-write_domain = obj-base.write_domain; err-fence_reg = obj-fence_reg; - err-pinned = 0; - if (i915_gem_obj_is_pinned(obj)) - err-pinned = 1; - if (obj-user_pin_count 0) + if (obj-user_pin_count 0) { + WARN_ON(i915_is_ggtt(vma-vm)); err-pinned = -1; + } else + err-pinned = !!vma-pin_count; err-tiling = obj-tiling_mode; err-dirty = obj-dirty; err-purgeable = obj-madv != I915_MADV_WILLNEED; @@ -666,7 +671,7 @@ static u32 capture_active_bo(struct drm_i915_error_buffer *err, int i = 0; list_for_each_entry(vma, head, mm_list) { - capture_bo(err++, vma-obj); + capture_vma(err++, vma); if (++i == count) break; } @@ -681,10 +686,10 @@ static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, int i = 0; list_for_each_entry(vma, head, pin_capture_link) { - if (!i915_gem_obj_is_pinned(vma-obj)) + if (!vma-pin_count) continue; - capture_bo(err++, vma-obj); + capture_vma(err++, vma); if (++i == count) break; } @@ -1083,16 +1088,16 @@ static void i915_gem_capture_buffers(struct drm_i915_private *dev_priv, struct drm_i915_error_state *error) { struct i915_address_space *vm; - int vm_count = 0, i = 0; + int i = 0; list_for_each_entry(vm, dev_priv-vm_list, global_link) - vm_count++; + error-vm_count++; - error-active_bo = kcalloc(vm_count, sizeof(*error-active_bo), GFP_ATOMIC); - error-pinned_bo = kcalloc(vm_count,
[Intel-gfx] [PATCH 07/14] drm/i915/error: Do a better job of disambiguating VMAs
From: Ben Widawsky benjamin.widaw...@intel.com Some of the original PPGTT patches in this area where unmerged, and this left a lot of confusion in our error capture with regard to which vm/obj we want to capture. There have been at least a couple of patches from Chris, and myself to try to fix this up; so here is another shot. Nobody running without full PPGTT is effected by this, and that is probably why nobody has bothered to fix it yet. Instead of using any of the global lists to find the VMAs we want to capture, we use the union of the active, and the inactive list in the VM. This allows us to replace our capture_bo with capture_vma, and know all the VMAs we want to capture are valid. I could have probably figured out a way to reuse mm_list. As we've had bugs here before in the shrinker, I think the best way forward is to get it working, and then optimize it later. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_gtt.c | 1 + drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++ drivers/gpu/drm/i915/i915_gpu_error.c | 39 ++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 5188936..0b2b982 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2115,6 +2115,7 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(vma-vma_link); + INIT_LIST_HEAD(vma-pin_capture_link); INIT_LIST_HEAD(vma-mm_list); INIT_LIST_HEAD(vma-exec_list); vma-vm = vm; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 8d6f7c1..1d75801 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -126,6 +126,8 @@ struct i915_vma { struct list_head vma_link; /* Link in the object's VMA list */ + struct list_head pin_capture_link; /* Link in the error capture */ + /** This vma's place in the batchbuffer or on the eviction list */ struct list_head exec_list; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ac101cd..4ff819e 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -677,14 +677,14 @@ static u32 capture_active_bo(struct drm_i915_error_buffer *err, static u32 capture_pinned_bo(struct drm_i915_error_buffer *err, int count, struct list_head *head) { - struct drm_i915_gem_object *obj; + struct i915_vma *vma; int i = 0; - list_for_each_entry(obj, head, global_list) { - if (!i915_gem_obj_is_pinned(obj)) + list_for_each_entry(vma, head, pin_capture_link) { + if (!i915_gem_obj_is_pinned(vma-obj)) continue; - capture_bo(err++, obj); + capture_bo(err++, vma-obj); if (++i == count) break; } @@ -1031,21 +1031,32 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv, const int vm_ndx) { struct drm_i915_error_buffer *active_bo = NULL, *pinned_bo = NULL; - struct drm_i915_gem_object *obj; struct i915_vma *vma; int active_vma_count = 0; + int vma_pin_count = 0; + LIST_HEAD(pinned_vma); - list_for_each_entry(vma, vm-active_list, mm_list) + list_for_each_entry(vma, vm-active_list, mm_list) { active_vma_count++; + if (vma-pin_count) { + vma_pin_count++; + list_move_tail(vma-pin_capture_link, pinned_vma); + } + } - error-active_bo_count[vm_ndx] = active_vma_count; - - list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) - if (i915_gem_obj_is_pinned(obj)) - active_vma_count++; + list_for_each_entry(vma, vm-inactive_list, mm_list) { + /* Certain objects may be on the inactive list, but pinned, when +* in the global GGTT. */ + if (WARN_ON(!i915_is_ggtt(vm) + vma-pin_count + !(vma-exec_entry-flags (131 { /* FIXME: need the actual flag */ + vma_pin_count++; + list_move_tail(vma-pin_capture_link, pinned_vma); + } + } - /* XXX: this is an incorrect measurement of pinned BOs */ - error-pinned_bo_count[vm_ndx] = active_vma_count - error-active_bo_count[vm_ndx]; + error-active_bo_count[vm_ndx] = active_vma_count; + error-pinned_bo_count[vm_ndx] = vma_pin_count; if (active_vma_count) { active_bo = kcalloc(active_vma_count, sizeof(*active_bo), GFP_ATOMIC); @@ -1063,7
[Intel-gfx] [PATCH 11/14] drm/i915: Reorder ctx unref on ppgtt cleanup
From: Ben Widawsky benjamin.widaw...@intel.com The comment [which was mine] is wrong. The context object can never be bound in a PPGTT because it is only capable of living in the Global GTT. So, remove the comment, and reorder the unref. What's nice about the latter is it keeps the context object alive past the PPGTT. This makes the destroy ordering symmetric with the creation ordering. Create: 1. Create context 2. Create PPGTT Destroy: 1. Destroy PPGTT 2. Destroy context As far as I know, this does not fix a bug. The code previously kept the context data structure, only the object was gone. As the code was, nothing tried to use the object after this point. NOTE: If in the future we have cases where the PPGTT can/should outlive the context (which doesn't occur today, but the code permits it), this ordering does not matter. Even if this occurs, as it stands now, we do not expect that to be the normal case, and having this order makes debugging a bit easier if we're tracking object lifetimes for the context vs ppgtt Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 0410bd9..70b3776 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -185,14 +185,12 @@ void i915_gem_context_free(struct kref *ctx_ref) /* We refcount even the aliasing PPGTT to keep the code symmetric */ if (USES_PPGTT(ctx-legacy_hw_ctx.rcs_state-base.dev)) ppgtt = ctx_to_ppgtt(ctx); - - /* XXX: Free up the object before tearing down the address space, in -* case we're bound in the PPGTT */ - drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base); } if (ppgtt) kref_put(ppgtt-ref, ppgtt_release); + if (ctx-legacy_hw_ctx.rcs_state) + drm_gem_object_unreference(ctx-legacy_hw_ctx.rcs_state-base); list_del(ctx-link); kfree(ctx); } -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/14] drm/i915/bdw: Enable full PPGTT
From: Ben Widawsky benjamin.widaw...@intel.com Broadwell is perfectly capable of full PPGTT. I've been using it for some time, and seen no especially ill effects. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0098ff9..f4f0bd1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2048,7 +2048,7 @@ struct drm_i915_cmd_table { #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)-gen = 6) #define HAS_ALIASING_PPGTT(dev)(INTEL_INFO(dev)-gen = 6) -#define HAS_PPGTT(dev) (INTEL_INFO(dev)-gen = 7 !IS_GEN8(dev)) +#define HAS_PPGTT(dev) (INTEL_INFO(dev)-gen = 7 !IS_CHERRYVIEW(dev)) #define USES_PPGTT(dev)intel_enable_ppgtt(dev, false) #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/14] drm/i915: Add some extra guards in evict_vm
From: Ben Widawsky benjamin.widaw...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_evict.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index bbf4b12..38297d3 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -214,6 +214,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle) struct i915_vma *vma, *next; int ret; + BUG_ON(!mutex_is_locked(vm-dev-struct_mutex)); trace_i915_gem_evict_vm(vm); if (do_idle) { @@ -222,11 +223,15 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle) return ret; i915_gem_retire_requests(vm-dev); + + WARN_ON(!list_empty(vm-active_list)); } - list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list) + list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list) { + WARN_ON(!i915_is_ggtt(vm) vma-pin_count); if (vma-pin_count == 0) WARN_ON(i915_vma_unbind(vma)); + } return 0; } -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/14] drm/i915: Make an uninterruptible evict
From: Ben Widawsky benjamin.widaw...@intel.com There are no users of this yet, but the idea is presented and split out to find bugs. Also, while here, return -ERESTARTSYS to the caller, in case they want to do something with it. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_drv.h| 2 +- drivers/gpu/drm/i915/i915_gem_context.c| 5 ++--- drivers/gpu/drm/i915/i915_gem_evict.c | 32 ++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 96cf5a9..a5ca462 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2523,7 +2523,7 @@ int __must_check i915_gem_evict_something(struct drm_device *dev, unsigned long start, unsigned long end, unsigned flags); -int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle); +int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle, bool interruptible); int i915_gem_evict_everything(struct drm_device *dev); /* belongs in i915_gem_gtt.h */ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index fab74d3..0410bd9 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -121,11 +121,10 @@ static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt) if (WARN_ON(list_empty(vma-vma_link) || list_is_singular(vma-vma_link))) break; - - i915_gem_evict_vm(ppgtt-base, true); + i915_gem_evict_vm(ppgtt-base, true, true); } else { i915_gem_retire_requests(dev); - i915_gem_evict_vm(ppgtt-base, false); + i915_gem_evict_vm(ppgtt-base, false, true); } ppgtt-base.cleanup(ppgtt-base); diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 38297d3..ac8d90f 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -197,8 +197,9 @@ found: /** * i915_gem_evict_vm - Evict all idle vmas from a vm * - * @vm: Address space to cleanse - * @do_idle: Boolean directing whether to idle first. + * @vm:Address space to cleanse + * @do_idle: Boolean directing whether to idle first. + * @interruptible: How to wait * * This function evicts all idles vmas from a vm. If all unpinned vmas should be * evicted the @do_idle needs to be set to true. @@ -209,18 +210,24 @@ found: * To clarify: This is for freeing up virtual address space, not for freeing * memory in e.g. the shrinker. */ -int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle) +int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle, bool interruptible) { + struct drm_i915_private *dev_priv = to_i915(vm-dev); + struct i915_vma *vma, *next; + bool was_intr = dev_priv-mm.interruptible; int ret; BUG_ON(!mutex_is_locked(vm-dev-struct_mutex)); trace_i915_gem_evict_vm(vm); + if (!interruptible) + dev_priv-mm.interruptible = false; + if (do_idle) { ret = i915_gpu_idle(vm-dev); if (ret) - return ret; + goto out; i915_gem_retire_requests(vm-dev); @@ -229,11 +236,20 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle) list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list) { WARN_ON(!i915_is_ggtt(vm) vma-pin_count); - if (vma-pin_count == 0) - WARN_ON(i915_vma_unbind(vma)); + if (vma-pin_count == 0) { + ret = i915_vma_unbind(vma); + if (ret == -ERESTARTSYS) { + BUG_ON(!interruptible); + goto out; + } else if (ret) + WARN(1, Failed to unbind vma %d\n, ret); + } } + ret = 0; - return 0; +out: + dev_priv-mm.interruptible = was_intr; + return ret; } /** @@ -276,7 +292,7 @@ i915_gem_evict_everything(struct drm_device *dev) /* Having flushed everything, unbind() should never raise an error */ list_for_each_entry(vm, dev_priv-vm_list, global_link) - WARN_ON(i915_gem_evict_vm(vm, false)); + WARN_ON(i915_gem_evict_vm(vm, false, true)); return 0; } diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 60998fc..1420aeb 100644 ---
[Intel-gfx] [PATCH 12/14] drm/i915: More correct (slower) ppgtt cleanup
From: Ben Widawsky benjamin.widaw...@intel.com If a VM still have objects which are bound (exactly: have a node reserved in the drm_mm), and we are in the middle of a reset, we have no hope of the standard methods fixing the situation (ring idle won't work). We must therefore let the reset handler take it's course, and then we can resume tearing down the VM. This logic very much duplicates^Wresembles the logic in our wait for error code. I've decided to leave it as open coded because I expect this bit of code to require tweaks and changes over time. Interruptions via signal causes a really similar problem. This should deprecate the need for the yet unmerged patch from Chris (and an identical patch from me, which was first!!): drm/i915: Prevent signals from interrupting close() I have a followup patch to implement deferred free, before you complain. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 51 +++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 70b3776..84c65aa 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -101,6 +101,32 @@ static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt) struct drm_device *dev = ppgtt-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; struct i915_address_space *vm = ppgtt-base; + bool do_idle = false; + int ret; + + /* If we get here while in reset, we need to let the reset handler run +* first, or else our VM teardown isn't going to go smoothly. There are +* a could of options at this point, but letting the reset handler do +* it's thing is the most desirable. The reset handler will take care of +* retiring the stuck requests. +*/ + if (i915_reset_in_progress(dev_priv-gpu_error)) { + mutex_unlock(dev-struct_mutex); +#define EXIT_COND (!i915_reset_in_progress(dev_priv-gpu_error) || \ + i915_terminally_wedged(dev_priv-gpu_error)) + ret = wait_event_timeout(dev_priv-gpu_error.reset_queue, +EXIT_COND, +10 * HZ); + if (!ret) { + /* it's unlikely idling will solve anything, but it +* shouldn't hurt to try. */ + do_idle = true; + /* TODO: go down kicking and screaming harder */ + } +#undef EXIT_COND + + mutex_lock(dev-struct_mutex); + } if (ppgtt == dev_priv-mm.aliasing_ppgtt || (list_empty(vm-active_list) list_empty(vm-inactive_list))) { @@ -117,14 +143,33 @@ static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt) if (!list_empty(vm-active_list)) { struct i915_vma *vma; + do_idle = true; list_for_each_entry(vma, vm-active_list, mm_list) if (WARN_ON(list_empty(vma-vma_link) || list_is_singular(vma-vma_link))) break; - i915_gem_evict_vm(ppgtt-base, true, true); - } else { + } else i915_gem_retire_requests(dev); - i915_gem_evict_vm(ppgtt-base, false, true); + + /* We have a problem here where VM teardown cannot be interrupted, or +* else the ppgtt cleanup will fail. As an example, a precisely timed +* SIGKILL could leads to an OOPS, or worse. There are two options: +* 1. Make the eviction uninterruptible +* 2. Defer the eviction if it was interrupted. +* +* Option #1 is not the friendliest, but it's the easiest to implement, +* and least error prone. +* TODO: Implement option 2 +*/ + ret = i915_gem_evict_vm(ppgtt-base, do_idle, !do_idle); + if (ret == -ERESTARTSYS) + ret = i915_gem_evict_vm(ppgtt-base, do_idle, false); + WARN_ON(ret); + WARN_ON(!list_empty(vm-active_list)); + + /* This is going to blow up badly if the mm is unclean */ + if (WARN_ON(!list_empty(ppgtt-base.mm.head_node.node_list))) { + /* TODO: go down kicking and screaming harder++ */ } ppgtt-base.cleanup(ppgtt-base); -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/14] drm/i915: Defer PPGTT cleanup
From: Ben Widawsky benjamin.widaw...@intel.com The last patch made PPGTT free cases correct. It left a major problem though where in many cases it was possible to have to idle the GPU in order to destroy a VM. This is really unfortunate as it is stalling the active GPU process for the dying GPU process. The workqueue grew very tricky. I left in my original wait based version as #if 0, and used Chris' recommendation with the seqno check. I haven't measure one vs the other, but I am in favor of the code as it is. I am just leaving the old code for people to observe. NOTE: I somewhat expect this patch to not get merged because of work in the pipeline. I won't argue much against that, and I'll simply say, this solves a real problem, or platforms running with PPGTT. PPGTT is not enabled by default yet, and I therefore see little harm in merging this. Because I was expecting this to not get merged, I did not spent much time investigating any corner cases, in particular, cases where I need to cleanup the wq. If this patch does head in the merge direction, I will take a closer look at that. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_drv.h | 10 +++ drivers/gpu/drm/i915/i915_gem.c | 110 drivers/gpu/drm/i915/i915_gem_context.c | 40 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.h | 2 + 5 files changed, 150 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a5ca462..0098ff9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1118,6 +1118,15 @@ struct i915_gem_mm { struct delayed_work idle_work; /** +* PPGTT freeing often happens during interruptible times (fd close, +* execbuf, busy_ioctl). It therefore becomes difficult to clean up the +* PPGTT when the refcount reaches 0 if a signal comes in. This +* workqueue defers the cleanup of the VM to a later time, and allows +* userspace to continue on. +*/ + struct delayed_work ppgtt_work; + + /** * Are we in a non-interruptible section of code like * modesetting? */ @@ -1502,6 +1511,7 @@ struct drm_i915_private { struct mutex modeset_restore_lock; struct list_head vm_list; /* Global list of all address spaces */ + struct list_head ppgtt_free_list; struct i915_gtt gtt; /* VM representing the global address space */ struct i915_gem_mm mm; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..2df4d86 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2707,6 +2707,112 @@ i915_gem_idle_work_handler(struct work_struct *work) intel_mark_idle(dev_priv-dev); } +static void +i915_gem_ppgtt_work_handler(struct work_struct *work) +{ + struct drm_i915_private *dev_priv = + container_of(work, typeof(*dev_priv), mm.ppgtt_work.work); + struct i915_address_space *vm, *v; +#if 0 + unsigned reset_counter; + int ret; +#endif + + if (!mutex_trylock(dev_priv-dev-struct_mutex)) { + queue_delayed_work(dev_priv-wq, dev_priv-mm.ppgtt_work, + round_jiffies_up_relative(HZ)); + return; + } + + list_for_each_entry_safe(vm, v, dev_priv-ppgtt_free_list, global_link) { + struct i915_hw_ppgtt *ppgtt = container_of(vm, struct i915_hw_ppgtt, base); + struct i915_vma *vma = NULL, *vma_active = NULL, *vma_inactive = NULL; + + /* The following attempts to find the newest (most recently +* activated/inactivated) VMA. +*/ + if (!list_empty(ppgtt-base.active_list)) { + vma_active = list_last_entry(ppgtt-base.active_list, +typeof(*vma), mm_list); + } + if (!list_empty(ppgtt-base.inactive_list)) { + vma_inactive = list_last_entry(ppgtt-base.inactive_list, + typeof(*vma), mm_list); + } + + if (vma_active) + vma = vma_active; + else if (vma_inactive) + vma = vma_inactive; + + /* Sanity check */ + if (vma_active vma_inactive) { + if (WARN_ON(vma_active-retire_seqno = vma_inactive-retire_seqno)) + vma = vma_inactive; + } + + if (!vma) + goto finish; + + /* Another sanity check */ + if (WARN_ON(!vma-retire_seqno)) + continue; + + /* NOTE: We could wait here for the seqno, but that makes
Re: [Intel-gfx] [PATCH 2/5] drm/i915: HSW_BLC_PWM2_CTL doesn't exist on BDW
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Fri, Jul 4, 2014 at 7:50 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So don't write it, otherwise we will trigger unclaimed register errors. Testcase: igt/pm_rpm/rte Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- 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 c12a5da..14505a1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7345,8 +7345,9 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv) WARN(I915_READ(PCH_PP_STATUS) PP_ON, Panel power on\n); WARN(I915_READ(BLC_PWM_CPU_CTL2) BLM_PWM_ENABLE, CPU PWM1 enabled\n); - WARN(I915_READ(HSW_BLC_PWM2_CTL) BLM_PWM_ENABLE, -CPU PWM2 enabled\n); + if (IS_HASWELL(dev)) + WARN(I915_READ(HSW_BLC_PWM2_CTL) BLM_PWM_ENABLE, +CPU PWM2 enabled\n); WARN(I915_READ(BLC_PWM_PCH_CTL1) BLM_PCH_PWM_ENABLE, PCH PWM1 enabled\n); WARN(I915_READ(UTIL_PIN_CTL) UTIL_PIN_ENABLE, -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: don't write powered down IRQ registers on Gen 8
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Thu, Jul 10, 2014 at 12:31 PM, Paulo Zanoni przan...@gmail.com wrote: 2014-07-08 11:58 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Tue, Jul 08, 2014 at 11:15:03AM -0300, Paulo Zanoni wrote: 2014-07-07 18:23 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Fri, Jul 04, 2014 at 11:50:29AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com If we enable unclaimed register reporting on Gen 8, we will discover that the IRQ registers for pipes B and C are also on the power well, so writes to them when the power well is disabled result in unclaimed register errors. Also, hsw_power_well_post_enable() already takes care of re-enabling them once the power well is enabled. Testcase: igt/pm_rpm/rte Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, shouldn't we split this into only setting up pipe A here and the pipe B stuff once we fire up the power well? No because these functions might be called when the power wells are already enabled. Hm, where does this still happen? bdw has power well support and chv has a different display block ... At driver init time... If you load i915.ko and the power wells are already enabled, we have to do it here. This code changed too often and I have no idea any more what's up and what's down here ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: extract and improve gen8_irq_power_well_post_enable
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Fri, Jul 4, 2014 at 7:50 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Move it from hsw_power_well_post_enable() (intel_pm.c) to i915_irq.c so we can reuse the nice IRQ macros we have there. The main difference is that now we're going to check if the IIR register is non-zero when we try to re-enable the interrupts. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 12 drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 18 ++ 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e116e9d..a8b8b6b 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3204,6 +3204,18 @@ static void gen8_irq_reset(struct drm_device *dev) ibx_irq_reset(dev); } +void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv) +{ + unsigned long irqflags; + + spin_lock_irqsave(dev_priv-irq_lock, irqflags); + GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B, dev_priv-de_irq_mask[PIPE_B], + ~dev_priv-de_irq_mask[PIPE_B]); + GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C, dev_priv-de_irq_mask[PIPE_C], + ~dev_priv-de_irq_mask[PIPE_C]); + spin_unlock_irqrestore(dev_priv-irq_lock, irqflags); +} + static void cherryview_irq_preinstall(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5f7c7bd..46a3a09 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -687,6 +687,7 @@ void intel_runtime_pm_disable_interrupts(struct drm_device *dev); void intel_runtime_pm_restore_interrupts(struct drm_device *dev); int intel_get_crtc_scanline(struct intel_crtc *crtc); void i9xx_check_fifo_underruns(struct drm_device *dev); +void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv); /* intel_crt.c */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 31ae2b4..4cc9e5c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5913,7 +5913,6 @@ bool intel_display_power_enabled(struct drm_i915_private *dev_priv, static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; - unsigned long irqflags; /* * After we re-enable the power well, if we touch VGA register 0x3d5 @@ -5929,21 +5928,8 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv) outb(inb(VGA_MSR_READ), VGA_MSR_WRITE); vga_put(dev-pdev, VGA_RSRC_LEGACY_IO); - if (IS_BROADWELL(dev)) { - spin_lock_irqsave(dev_priv-irq_lock, irqflags); - I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_B), - dev_priv-de_irq_mask[PIPE_B]); - I915_WRITE(GEN8_DE_PIPE_IER(PIPE_B), - ~dev_priv-de_irq_mask[PIPE_B] | - GEN8_PIPE_VBLANK); - I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_C), - dev_priv-de_irq_mask[PIPE_C]); - I915_WRITE(GEN8_DE_PIPE_IER(PIPE_C), - ~dev_priv-de_irq_mask[PIPE_C] | - GEN8_PIPE_VBLANK); - POSTING_READ(GEN8_DE_PIPE_IER(PIPE_C)); - spin_unlock_irqrestore(dev_priv-irq_lock, irqflags); - } + if (IS_BROADWELL(dev)) + gen8_irq_power_well_post_enable(dev_priv); } static void hsw_set_power_well(struct drm_i915_private *dev_priv, -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: reorganize the unclaimed register detection code
On Mon, Jul 7, 2014 at 2:34 PM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Jul 04, 2014 at 11:50:32AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The current code only runs when we do an I915_WRITE operation. It checks if the unclaimed register flag is set before we do the operation, and then it checks it again after we do the operation. This double check allows us to find out if the I915_WRITE operation in question is the bad one, or if some previous code is the bad one. When it finds a problem, our code uses DRM_ERROR to signal it. The good thing about the current code is that it detects the problem, so at least we can know we did something wrong. The problem is that even though we find the problem, we don't really have much information to actually debug it. So whenever I see one of these DRM_ERROR messages on my systems, the first thing I do is apply a patch to change the DRM_ERROR to a WARN and also check for unclaimed registers on I915_READ operations. This local patch makes things even slower, but it usually helps a lot in finding the bad code. The first point here is that since the current code is only useful to detect whether we have a problem or not, but it is not really good to find the cause of the problem, I don't think we should be checking both before and after every I915_WRITE operation: just doing the check once should be enough for us to quickly detect problems. With this change, the code that runs by default for every single user will only do 1 read operation for every single I915_WRITE, instead of 2. This patch does this change. The second point is that the local patch I have should be upstream, but since it makes things slower it should be disabled by default. So I added the i915.mmio_debug option to enable it. So after this patch, this is what will happen: - By default, we will try to detect unclaimed registers once after every I915_WRITE operation. Previously we tried twice for every I915_WRITE. - When we find an unclaimed register we will still print a DRM_ERROR message, but we will now tell the user to try again with i915.mmio_debug=1. - When we use i915.mmio_debug=1 we will try to find unclaimed registers both before and after every I915_READ and I915_WRITE operation, and we will print stack traces in case we find them. This should really help locating the exact point of the bad code (or at least finding out that i915.ko is not the problem). This commit also opens space for really-slow register debugging operations on other platforms. In theory we can now add lots and lots of debug code behind i915.mmio_debug, enable this option on our tests, and catch more problems. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_params.c | 6 drivers/gpu/drm/i915/intel_uncore.c | 68 + 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ac06c0f..51d867f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2092,6 +2092,7 @@ struct i915_params { bool disable_display; bool disable_vtd_wa; int use_mmio_flip; + bool mmio_debug; }; extern struct i915_params i915 __read_mostly; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 8145729..7977872 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -49,6 +49,7 @@ struct i915_params i915 __read_mostly = { .enable_cmd_parser = 1, .disable_vtd_wa = 0, .use_mmio_flip = 0, + .mmio_debug = 0, }; module_param_named(modeset, i915.modeset, int, 0400); @@ -161,3 +162,8 @@ MODULE_PARM_DESC(enable_cmd_parser, module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600); MODULE_PARM_DESC(use_mmio_flip, use MMIO flips (-1=never, 0=driver discretion [default], 1=always)); + +module_param_named(mmio_debug, i915.mmio_debug, bool, 0600); +MODULE_PARM_DESC(disable_vtd_wa, wrong parameter here! + Enable the MMIO debug code (default: false). This may negatively + affect performance.); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 29145df..de5402f 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -513,21 +513,72 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) __raw_i915_write32(dev_priv, MI_MODE, 0); } +/** + * hsw_unclaimed_reg_debug - tries to find unclaimed registers + * @dev_priv: device private data + * @reg: register we're about to touch or just have touched + * @read: are we reading or writing a register now?
Re: [Intel-gfx] [PATCH 5/5] drm/i915: BDW can also detect unclaimed registers
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Fri, Jul 4, 2014 at 7:50 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com By the time I wrote this patch, it allowed me to catch some problems. But due to patch reordering - in order to prevent fake regression reports - this patch may be merged after the fixes of the problems identified by this patch. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 drivers/gpu/drm/i915/intel_uncore.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8a0cb0c..bdb223c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -303,6 +303,7 @@ static const struct intel_device_info intel_broadwell_d_info = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, .has_llc = 1, .has_ddi = 1, + .has_fpga_dbg = 1, .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, @@ -314,6 +315,7 @@ static const struct intel_device_info intel_broadwell_m_info = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, .has_llc = 1, .has_ddi = 1, + .has_fpga_dbg = 1, .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, @@ -325,6 +327,7 @@ static const struct intel_device_info intel_broadwell_gt3d_info = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, .has_llc = 1, .has_ddi = 1, + .has_fpga_dbg = 1, .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, @@ -336,6 +339,7 @@ static const struct intel_device_info intel_broadwell_gt3m_info = { .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, .has_llc = 1, .has_ddi = 1, + .has_fpga_dbg = 1, .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index de5402f..1fcf78b 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -788,6 +788,7 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, u32 reg) static void \ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \ REG_WRITE_HEADER; \ + hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ if (reg 0x4 !is_gen8_shadowed(dev_priv, reg)) { \ if (dev_priv-uncore.forcewake_count == 0) \ dev_priv-uncore.funcs.force_wake_get(dev_priv, \ @@ -799,6 +800,8 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace } else { \ __raw_i915_write##x(dev_priv, reg, val); \ } \ + hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ + hsw_unclaimed_reg_detect(dev_priv); \ REG_WRITE_FOOTER; \ } -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not access stolen memory directly by the CPU, even for error capture
On Tue, Jul 15, 2014 at 04:15:08PM +0200, Daniel Vetter wrote: On Thu, Apr 24, 2014 at 02:47:48PM -0700, Ben Widawsky wrote: On Wed, Feb 12, 2014 at 07:18:40PM +, Chris Wilson wrote: For stolen pages, since it is verboten to access them directly on many architectures, we have to read them through the GTT aperture. If they are not accessible through the aperture, then we have to abort. This was complicated by commit 8b6124a633d8095b0c8364f585edff9c59568a96 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Thu Jan 30 14:38:16 2014 + drm/i915: Don't access snooped pages through the GTT (even for error capture) and the desire to use stolen memory for ringbuffers, contexts and batches in the future. I am somewhat unclear as to whether we want to prefer the aperture for reading back objects which may be mapped in multiple address spaces. Can we just ioremap the physical address (at least for error capture)? Is there a polished version of this floating somewhere which I've missed? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, 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/chv: Use timeout mode for RC6 on chv
From: Deepak S deepa...@linux.intel.com Higher RC6 residency is observed using timeout mode instead of EI mode. It's Recommended to use TO Method for RC6. Signed-off-by: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 19c5c26..88bad36 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4037,7 +4037,7 @@ static void cherryview_enable_rps(struct drm_device *dev) I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10); I915_WRITE(GEN6_RC_SLEEP, 0); - I915_WRITE(GEN6_RC6_THRESHOLD, 5); /* 50/125ms per EI */ + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557); /* allows RC6 residency counter to work */ I915_WRITE(VLV_COUNTER_CONTROL, @@ -4053,7 +4053,7 @@ static void cherryview_enable_rps(struct drm_device *dev) /* 3: Enable RC6 */ if ((intel_enable_rc6(dev) INTEL_RC6_ENABLE) (pcbr VLV_PCBR_ADDR_SHIFT)) - rc6_mode = GEN6_RC_CTL_EI_MODE(1); + rc6_mode = GEN7_RC_CTL_TO_MODE; I915_WRITE(GEN6_RC_CONTROL, rc6_mode); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx