Re: [Intel-gfx] [PATCH 00/10] Enable RC6/Turbo on CHV
On Wednesday 23 April 2014 01:49 AM, Daniel Vetter wrote: On Mon, Apr 21, 2014 at 07:28:54PM +0530, Deepak S wrote: Hi Ville, let me know if you want some of other small patches to be squashed. Quick aside: Something seems to have gone with git send-email thread - the patches aren't in-reply-to the cover letter. No idea how that happened though ... -Daniel Not Sure :( I can resend the patches again Thanks Deepak On Monday 21 April 2014 01:23 PM, deepa...@linux.intel.com wrote: From: Deepak S Squashed some of the patches and created a new patch series. ToDo: Address the comments on some the patches. Changes will be shared in next series. Ben Widawsky (1): drm/i915/bdw: Implement a basic PM interrupt handler Deepak S (6): drm/i915: Enable PM Interrupts target via Display Interface. drm/i915/chv: Enable Render Standby (RC6) for Cheeryview drm/i915/chv: Added CHV specific register read and write drm/i915/chv: Enable RPS (Turbo) for Cheeryview drm/i915/chv: Added CHV specific DDR fetch into init_clock_gating drm/i915/chv: Freq(opcode) request value for CHV. Ville Syrjälä (3): drm/i915/chv: Streamline CHV forcewake stuff drm/i915/chv: CHV doesn't need WaRsForcewakeWaitTC0 drm/i915/chv: Skip gen6_gt_check_fifodbg() on CHV drivers/gpu/drm/i915/i915_drv.h | 10 ++ drivers/gpu/drm/i915/i915_irq.c | 79 +++- drivers/gpu/drm/i915/i915_reg.h | 13 ++ drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 231 +- drivers/gpu/drm/i915/intel_sideband.c | 15 +++ drivers/gpu/drm/i915/intel_uncore.c | 126 +-- 7 files changed, 455 insertions(+), 21 deletions(-) ___ 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 I-g-t V3 0/2] Tests: Add test cases based on multi drm_fd to test sync
This follows Daniel's advice to add the two test cases based on multi drm_fd to test the ring sync and CPU<->GPU sync. The Broadwell GT3 machine has two independent BSD rings that can be used to process the video commands. This is implemented in kernel driver and transparent to the user-space. But we still need to check the ring sync and CPU<->GPU sync for the second BSD ring. Two tests are created based on the multi drm_fds to test the sync. Multi drm_fd can assure that the second BSD ring has the opportunity to dispatch the GPU command. V1->V2: Follow Daniel's comment to add one subtext instead of one individual test case, which is used to test the CPU<->GPU sync under multi BSD rings/ V2->V3: Follow Imre's comment to remove the unnecessary initialization and use igt_assert_f instead of igt_assert Zhao Yakui (2): tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU<->GPU sync under multi BSD rings tests/Makefile.sources |1 + tests/gem_dummy_reloc_loop.c| 107 +++- tests/gem_multi_bsd_sync_loop.c | 175 +++ 3 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 tests/gem_multi_bsd_sync_loop.c -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH I-g-t V3 2/2] tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU<->GPU sync under multi BSD rings
The Broadwell GT3 machine has two independent BSD rings in kernel driver while
it is transparent to the user-space driver. In such case it needs to check
the CPU<->GPU sync for the second BSD ring.
V1->V2: Follow Daniel's comment to add one subtext instead of one individual
test case, which is used to test the CPU<->GPU sync under multi BSD rings.
V2->V3: Follow Imre's comment to remove the unnecessary initialization and
use igt_assert_f instead of igt_assert
Reviewed-by: Imre Deak
Signed-off-by: Zhao Yakui
---
tests/gem_dummy_reloc_loop.c | 107 +-
1 file changed, 106 insertions(+), 1 deletion(-)
diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
index a61b59b..4e4dd49 100644
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -48,6 +48,13 @@ static drm_intel_bufmgr *bufmgr;
struct intel_batchbuffer *batch;
static drm_intel_bo *target_buffer;
+#define NUM_FD 50
+
+static int mfd[NUM_FD];
+static drm_intel_bufmgr *mbufmgr[NUM_FD];
+static struct intel_batchbuffer *mbatch[NUM_FD];
+static drm_intel_bo *mbuffer[NUM_FD];
+
/*
* Testcase: Basic check of ring<->cpu sync using a dummy reloc
*
@@ -124,6 +131,50 @@ dummy_reloc_loop_random_ring(int num_rings)
}
}
+static void
+dummy_reloc_loop_random_ring_multi_fd(int num_rings)
+{
+ int i;
+ struct intel_batchbuffer *saved_batch;
+
+ saved_batch = batch;
+
+ srandom(0xdeadbeef);
+
+ for (i = 0; i < 0x10; i++) {
+ int mindex;
+ int ring = random() % num_rings + 1;
+
+ mindex = random() % NUM_FD;
+ batch = mbatch[mindex];
+
+ if (ring == I915_EXEC_RENDER) {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
+ OUT_BATCH(0x); /* compare dword */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+ } else {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_FLUSH_DW | 1);
+ OUT_BATCH(0); /* reserved */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
+ ADVANCE_BATCH();
+ }
+ intel_batchbuffer_flush_on_ring(batch, ring);
+
+ drm_intel_bo_map(target_buffer, 0);
+ // map to force waiting on rendering
+ drm_intel_bo_unmap(target_buffer);
+ }
+
+ batch = saved_batch;
+}
+
int fd;
int devid;
int num_rings;
@@ -133,6 +184,7 @@ igt_main
igt_skip_on_simulation();
igt_fixture {
+ int i;
fd = drm_open_any();
devid = intel_get_drm_devid(fd);
num_rings = gem_get_num_rings(fd);
@@ -148,6 +200,40 @@ igt_main
target_buffer = drm_intel_bo_alloc(bufmgr, "target bo", 4096,
4096);
igt_assert(target_buffer);
+
+ /* Create multi drm_fd and map one gem object to multi
gem_contexts */
+ {
+ unsigned int target_flink;
+ char buffer_name[32];
+ if (dri_bo_flink(target_buffer, &target_flink)) {
+ printf("fail to get flink for target buffer\n");
+ igt_assert_f(0, "fail to create global "
+"gem_handle for target buffer\n");
+ }
+ for (i = 0; i < NUM_FD; i++) {
+ sprintf(buffer_name, "Target buffer %d\n", i);
+ mfd[i] = drm_open_any();
+ mbufmgr[i] = drm_intel_bufmgr_gem_init(mfd[i],
4096);
+ igt_assert_f(mbufmgr[i],
+"fail to initialize buf manager "
+"for drm_fd %d\n",
+mfd[i]);
+ drm_intel_bufmgr_gem_enable_reuse(mbufmgr[i]);
+ mbatch[i] = intel_batchbuffer_alloc(mbufmgr[i],
devid);
+ igt_assert_f(mbatch[i],
+"fail to create batchbuffer "
+"for drm_fd %d\n",
+mfd[i]);
+ mbuffer[i] = intel_bo_gem_create_from_name(
+ mbufmgr[i],
+
[Intel-gfx] [PATCH I-g-t V3 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
The Broadwell GT3 machine has two independent BSD rings in kernel driver while
it is transparent to the user-space driver. In such case it needs to check
the ring sync between the two BSD rings. At the same time it also needs to
check the sync among the second BSD ring and the other rings.
V2->V3: Follow Imre's comment to remove the unnecessary initialization and
use igt_assert_f instead of igt_assert.
Signed-off-by: Zhao Yakui
---
tests/Makefile.sources |1 +
tests/gem_multi_bsd_sync_loop.c | 175 +++
2 files changed, 176 insertions(+)
create mode 100644 tests/gem_multi_bsd_sync_loop.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..7cd9ca8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -105,6 +105,7 @@ TESTS_progs = \
gem_render_tiled_blits \
gem_ring_sync_copy \
gem_ring_sync_loop \
+ gem_multi_bsd_sync_loop \
gem_seqno_wrap \
gem_set_tiling_vs_gtt \
gem_set_tiling_vs_pwrite \
diff --git a/tests/gem_multi_bsd_sync_loop.c b/tests/gem_multi_bsd_sync_loop.c
new file mode 100644
index 000..b01764a
--- /dev/null
+++ b/tests/gem_multi_bsd_sync_loop.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Daniel Vetter (based on gem_ring_sync_loop_*.c)
+ *Zhao Yakui
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "intel_bufmgr.h"
+#include "intel_batchbuffer.h"
+#include "intel_io.h"
+#include "i830_reg.h"
+#include "intel_chipset.h"
+
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+static drm_intel_bo *target_buffer;
+
+#define NUM_FD 50
+
+static int mfd[NUM_FD];
+static drm_intel_bufmgr *mbufmgr[NUM_FD];
+static struct intel_batchbuffer *mbatch[NUM_FD];
+static drm_intel_bo *mbuffer[NUM_FD];
+
+
+/*
+ * Testcase: Basic check of ring<->ring sync using a dummy reloc
+ *
+ * Extremely efficient at catching missed irqs with semaphores=0 ...
+ */
+
+#define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
+#define MI_DO_COMPARE (1<<21)
+
+static void
+store_dword_loop(int fd)
+{
+ int i;
+ int num_rings = gem_get_num_rings(fd);
+
+ srandom(0xdeadbeef);
+
+ for (i = 0; i < SLOW_QUICK(0x10, 10); i++) {
+ int ring, mindex;
+ ring = random() % num_rings + 1;
+ mindex = random() % NUM_FD;
+ batch = mbatch[mindex];
+ if (ring == I915_EXEC_RENDER) {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
+ OUT_BATCH(0x); /* compare dword */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+ } else {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_FLUSH_DW | 1);
+ OUT_BATCH(0); /* reserved */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
+ ADVANCE_BATCH();
+ }
+ intel_batchbuffer_flush_on_ring(batch, ring);
+ }
+
+ drm_intel_bo_map(target_buffer, 0);
+ // map to force waiting on rendering
+ drm_intel_bo_unmap(target_buffer);
+}
+
+igt_simple_main
+{
+ int fd;
+ int devid;
+ int i;
+
+ fd = drm_open_any();
+ devid = intel_get_drm_devid(fd);
+ gem_require_ring(fd, I915_
Re: [Intel-gfx] [PATCH I-g-t V2 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
On Tue, 2014-04-22 at 13:44 -0600, Daniel Vetter wrote:
> On Tue, Apr 22, 2014 at 02:52:04PM +0300, Imre Deak wrote:
> > On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> > > The Broadwell GT3 machine has two independent BSD rings in kernel driver
> > > while
> > > it is transparent to the user-space driver. In such case it needs to check
> > > the ring sync between the two BSD rings. At the same time it also needs to
> > > check the sync among the second BSD ring and the other rings.
> > >
> > > Signed-off-by: Zhao Yakui
> > > ---
> > > tests/Makefile.sources |1 +
> > > tests/gem_multi_bsd_sync_loop.c | 172
> > > +++
> > > 2 files changed, 173 insertions(+)
> > > create mode 100644 tests/gem_multi_bsd_sync_loop.c
> > >
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index c957ace..7cd9ca8 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -105,6 +105,7 @@ TESTS_progs = \
> > > gem_render_tiled_blits \
> > > gem_ring_sync_copy \
> > > gem_ring_sync_loop \
> > > + gem_multi_bsd_sync_loop \
> > > gem_seqno_wrap \
> > > gem_set_tiling_vs_gtt \
> > > gem_set_tiling_vs_pwrite \
> > > diff --git a/tests/gem_multi_bsd_sync_loop.c
> > > b/tests/gem_multi_bsd_sync_loop.c
> > > new file mode 100644
> > > index 000..7f5b832
> > > --- /dev/null
> > > +++ b/tests/gem_multi_bsd_sync_loop.c
> > > @@ -0,0 +1,172 @@
> > > +/*
> > > + * Copyright © 2014 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining
> > > a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the
> > > next
> > > + * paragraph) shall be included in all copies or substantial portions of
> > > the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + * Authors:
> > > + *Daniel Vetter (based on
> > > gem_ring_sync_loop_*.c)
> > > + *Zhao Yakui
> > > + *
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include "drm.h"
> > > +#include "ioctl_wrappers.h"
> > > +#include "drmtest.h"
> > > +#include "intel_bufmgr.h"
> > > +#include "intel_batchbuffer.h"
> > > +#include "intel_io.h"
> > > +#include "i830_reg.h"
> > > +#include "intel_chipset.h"
> > > +
> > > +static drm_intel_bufmgr *bufmgr;
> > > +struct intel_batchbuffer *batch;
> > > +static drm_intel_bo *target_buffer;
> > > +
> > > +#define NUM_FD 50
> > > +
> > > +static int mfd[NUM_FD];
> > > +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> > > +static struct intel_batchbuffer *mbatch[NUM_FD];
> > > +static drm_intel_bo *mbuffer[NUM_FD];
> > > +
> > > +
> > > +/*
> > > + * Testcase: Basic check of ring<->ring sync using a dummy reloc
> > > + *
> > > + * Extremely efficient at catching missed irqs with semaphores=0 ...
> > > + */
> > > +
> > > +#define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
> > > +#define MI_DO_COMPARE(1<<21)
> > > +
> > > +static void
> > > +store_dword_loop(int fd)
> > > +{
> > > + int i;
> > > + int num_rings = gem_get_num_rings(fd);
> > > +
> > > + srandom(0xdeadbeef);
> > > +
> > > + for (i = 0; i < SLOW_QUICK(0x10, 10); i++) {
> > > + int ring, mindex;
> > > + ring = random() % num_rings + 1;
> > > + mindex = random() % NUM_FD;
> > > + batch = mbatch[mindex];
> > > + if (ring == I915_EXEC_RENDER) {
> > > + BEGIN_BATCH(4);
> > > + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> > > + OUT_BATCH(0x); /* compare dword */
> > > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > > + I915_GEM_DOMAIN_RENDER, 0);
> > > + OUT_BATCH(MI_NOOP);
> > > + ADVANCE_BATCH();
> > > + } else {
> > > + BEGIN_BATCH(4);
> > > +
Re: [Intel-gfx] [PATCH I-g-t V2 2/2] tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU<->GPU sync under multi BSD rings
On Tue, 2014-04-22 at 13:48 -0600, Daniel Vetter wrote:
> On Tue, Apr 22, 2014 at 03:05:03PM +0300, Imre Deak wrote:
> > On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> > > The Broadwell GT3 machine has two independent BSD rings in kernel driver
> > > while
> > > it is transparent to the user-space driver. In such case it needs to check
> > > the CPU<->GPU sync for the second BSD ring.
> > >
> > > V1->V2: Follow Daniel's comment to add one subtext instead of one
> > > individual
> > > test case, which is used to test the CPU<->GPU sync under multi BSD rings
> > >
> > > Signed-off-by: Zhao Yakui
> > > ---
> > > tests/gem_dummy_reloc_loop.c | 102
> > > +-
> > > 1 file changed, 101 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
> > > index a61b59b..660d8e1 100644
> > > --- a/tests/gem_dummy_reloc_loop.c
> > > +++ b/tests/gem_dummy_reloc_loop.c
> > > @@ -48,6 +48,13 @@ static drm_intel_bufmgr *bufmgr;
> > > struct intel_batchbuffer *batch;
> > > static drm_intel_bo *target_buffer;
> > >
> > > +#define NUM_FD 50
> > > +
> > > +static int mfd[NUM_FD];
> > > +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> > > +static struct intel_batchbuffer *mbatch[NUM_FD];
> > > +static drm_intel_bo *mbuffer[NUM_FD];
> > > +
> > > /*
> > > * Testcase: Basic check of ring<->cpu sync using a dummy reloc
> > > *
> > > @@ -124,6 +131,50 @@ dummy_reloc_loop_random_ring(int num_rings)
> > > }
> > > }
> > >
> > > +static void
> > > +dummy_reloc_loop_random_ring_multi_fd(int num_rings)
> > > +{
> > > + int i;
> > > + struct intel_batchbuffer *saved_batch;
> > > +
> > > + saved_batch = batch;
> > > +
> > > + srandom(0xdeadbeef);
> > > +
> > > + for (i = 0; i < 0x10; i++) {
> > > + int mindex;
> > > + int ring = random() % num_rings + 1;
> > > +
> > > + mindex = random() % NUM_FD;
> > > + batch = mbatch[mindex];
> > > +
> > > + if (ring == I915_EXEC_RENDER) {
> > > + BEGIN_BATCH(4);
> > > + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> > > + OUT_BATCH(0x); /* compare dword */
> > > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > > + I915_GEM_DOMAIN_RENDER, 0);
> > > + OUT_BATCH(MI_NOOP);
> > > + ADVANCE_BATCH();
> > > + } else {
> > > + BEGIN_BATCH(4);
> > > + OUT_BATCH(MI_FLUSH_DW | 1);
> > > + OUT_BATCH(0); /* reserved */
> > > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > > + I915_GEM_DOMAIN_RENDER, 0);
> > > + OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
> > > + ADVANCE_BATCH();
> > > + }
> > > + intel_batchbuffer_flush_on_ring(batch, ring);
> > > +
> > > + drm_intel_bo_map(target_buffer, 0);
> > > + // map to force waiting on rendering
> > > + drm_intel_bo_unmap(target_buffer);
> > > + }
> > > +
> > > + batch = saved_batch;
> > > +}
> > > +
> > > int fd;
> > > int devid;
> > > int num_rings;
> > > @@ -133,6 +184,7 @@ igt_main
> > > igt_skip_on_simulation();
> > >
> > > igt_fixture {
> > > + int i;
> > > fd = drm_open_any();
> > > devid = intel_get_drm_devid(fd);
> > > num_rings = gem_get_num_rings(fd);
> > > @@ -148,6 +200,35 @@ igt_main
> > >
> > > target_buffer = drm_intel_bo_alloc(bufmgr, "target bo", 4096,
> > > 4096);
> > > igt_assert(target_buffer);
> > > +
> > > + /* Create multi drm_fd and map one gem object to multi
> > > gem_contexts */
> > > + {
> > > + unsigned int target_flink;
> > > + char buffer_name[32];
> > > + if (dri_bo_flink(target_buffer, &target_flink)) {
> > > + printf("fail to get flink for target buffer\n");
> > > + igt_assert(0);
> >
> > For the future: could be just igt_assert_f().
>
> Yeah I think for new testcases we should try to use the latest igt_*
> macros and helpers as much as possible. Reducing control flow and
> replacing it by the right igt_assert/require/... macro imo really helps
> the readability of testcases.
Hi, Daniel/Imre
Thanks for your comments and advice.
I will update it.
Thanks.
Yakui
> -Daniel
> >
> > > + }
> > > + for (i = 0; i < NUM_FD; i++) {
> > > + mfd[i] = 0;
> > > + mbufmgr[i] = NULL;
> > > + mbuffer[i] = NULL;
> > > + }
> >
> > Nitpick: the above are all statics, so no need to init them.
> >
> > Other than the above this looks good:
> > Reviewed-by: Imre Deak
> >
> > > + for (i = 0; i < NUM_FD; i++) {
> > > + s
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Invalidate our pages under memory pressure
On 22 April 2014 20:06:14, Daniel Vetter wrote:
On Thu, Apr 17, 2014 at 07:34:31PM +0300, Jani Nikula wrote:
On Wed, 16 Apr 2014, Robert Beckett wrote:
On 25/03/2014 13:23, Chris Wilson wrote:
Try to flush out dirty pages into the swapcache (and from there into the
swapfile) when under memory pressure and forced to drop GEM objects from
memory. In effect, this should just allow us to discard unused pages for
memory reclaim and to start writeback earlier.
v2: Hugh Dickins warned that explicitly starting writeback from
shrink_slab was prone to deadlocks within shmemfs.
Cc: Hugh Dickins
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c | 38 +++---
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 135ee8bd55f6..8287fd6701c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -60,7 +60,6 @@ static unsigned long i915_gem_shrinker_scan(struct shrinker
*shrinker,
struct shrink_control *sc);
static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long
target);
static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
-static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
static void i915_gem_retire_requests_ring(struct intel_ring_buffer *ring);
static bool cpu_cache_is_coherent(struct drm_device *dev,
@@ -1685,12 +1684,16 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void
*data,
return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
}
+static inline int
+i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
+{
+ return obj->madv == I915_MADV_DONTNEED;
+}
+
/* Immediately discard the backing storage */
static void
i915_gem_object_truncate(struct drm_i915_gem_object *obj)
{
- struct inode *inode;
-
i915_gem_object_free_mmap_offset(obj);
if (obj->base.filp == NULL)
@@ -1701,16 +1704,28 @@ i915_gem_object_truncate(struct drm_i915_gem_object
*obj)
* To do this we must instruct the shmfs to drop all of its
* backing pages, *now*.
*/
- inode = file_inode(obj->base.filp);
- shmem_truncate_range(inode, 0, (loff_t)-1);
-
+ shmem_truncate_range(file_inode(obj->base.filp), 0, (loff_t)-1);
obj->madv = __I915_MADV_PURGED;
}
-static inline int
-i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
+/* Try to discard unwanted pages */
+static void
+i915_gem_object_invalidate(struct drm_i915_gem_object *obj)
{
- return obj->madv == I915_MADV_DONTNEED;
+ struct address_space *mapping;
+
+ switch (obj->madv) {
+ case I915_MADV_DONTNEED:
+ i915_gem_object_truncate(obj);
+ case __I915_MADV_PURGED:
+ return;
+ }
+
+ if (obj->base.filp == NULL)
+ return;
+
+ mapping = file_inode(obj->base.filp)->i_mapping,
+ invalidate_mapping_pages(mapping, 0, (loff_t)-1);
}
static void
@@ -1775,8 +1790,7 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
ops->put_pages(obj);
obj->pages = NULL;
- if (i915_gem_object_is_purgeable(obj))
- i915_gem_object_truncate(obj);
+ i915_gem_object_invalidate(obj);
return 0;
}
@@ -4201,6 +4215,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
if (WARN_ON(obj->pages_pin_count))
obj->pages_pin_count = 0;
+ if (obj->madv != __I915_MADV_PURGED)
+ obj->madv = I915_MADV_DONTNEED;
i915_gem_object_put_pages(obj);
i915_gem_object_free_mmap_offset(obj);
i915_gem_object_release_stolen(obj);
Functionally it looks good to me.
Though, you may want a /* fall-through */ comment (some people cant
mentally parse fallthroughs without being prompted) and a default:
break; (to avoid any static code analysis complaints) in the switch in
i915_gem_object_invalidate.
Or just two if statements.
if (MADV_DONTNEED)
i915_gem_object_truncate(obj);
if (!MADV_WILLNEED)
return;
would indeed looka a bit cleaner.
And a question to Bob: Is your r-b for the entire series or just this
patch? Generally the assumption is that an r-b tag is only for the patch
replied to, except when otherwise stated. Usually people just slap r-b
tags onto patches as they go through a series.
-Daniel
It was meant for the whole series.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: get power domain in case the BIOS enabled eDP VDD
From: Paulo Zanoni
If I unplug the eDP monitor, the BIOS of my machine will enable the
VDD bit, then when the driver loads it will think VDD is enabled. It
will detect that the eDP is not enabled and return false from
intel_edp_init_connector. This will trigger a call to
edp_panel_vdd_off_sync(), which trigger a WARN saying that the
refcount of the power domain is less than zero.
The problem happens because the driver gets a refcount whenever it
enables the VDD bit, and puts the refcount whenever it disables the
VDD bit. But on this case, the BIOS enabled VDD, so all we do is to
call put() without calling get() first, so the code added is there to
make sure we always have the get() in case the BIOS enabled the bit.
v2: - Rebase
Tested-by: Chris Wilson (v1)
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_dp.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 80e5598..44df493 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3785,7 +3785,8 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
{
struct drm_connector *connector = &intel_connector->base;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct intel_encoder *intel_encoder = &intel_dig_port->base;
+ struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *fixed_mode = NULL;
struct drm_display_mode *downclock_mode = NULL;
@@ -3798,6 +3799,14 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
if (!is_edp(intel_dp))
return true;
+ /* The VDD bit needs a power domain reference, so if the bit is already
+* enabled when we boot, grab this reference. */
+ if (edp_have_panel_vdd(intel_dp)) {
+ enum intel_display_power_domain power_domain;
+ power_domain = intel_display_port_power_domain(intel_encoder);
+ intel_display_power_get(dev_priv, power_domain);
+ }
+
/* Cache DPCD and EDID for edp. */
intel_edp_panel_vdd_on(intel_dp);
has_dpcd = intel_dp_get_dpcd(intel_dp);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: remove redundant is_edp() check
From: Paulo Zanoni
If intel_edp_init_connector() returns false, then we know that
is_edp() is true because of the early return at
intel_edp_init_connector(). So remove the redundant check.
Change proposed by Chris on his review to "drm/i915: get power domain
in case the BIOS enabled eDP VDD".
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_dp.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b385b03..a25f708 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3958,12 +3958,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) {
drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
- if (is_edp(intel_dp)) {
- cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
- mutex_lock(&dev->mode_config.mutex);
- edp_panel_vdd_off_sync(intel_dp);
- mutex_unlock(&dev->mode_config.mutex);
- }
+ cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
+ mutex_lock(&dev->mode_config.mutex);
+ edp_panel_vdd_off_sync(intel_dp);
+ mutex_unlock(&dev->mode_config.mutex);
drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
return false;
--
1.9.0
___
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 intel_dp_power_get/put
From: Paulo Zanoni
This was suggested by Chris on his review to the first version of
"drm/i915: get power domain in case the BIOS enabled eDP VDD". Well,
at least that's what I understood from his comment :)
The downside of this patch is that in a few places we will call
intel_display_port_power_domain() twice instead of once.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_dp.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44df493..b385b03 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1063,13 +1063,30 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
return control;
}
+static void intel_dp_power_get(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ enum intel_display_power_domain power_domain;
+
+ power_domain = intel_display_port_power_domain(encoder);
+ intel_display_power_get(dev_priv, power_domain);
+}
+
+static void intel_dp_power_put(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ enum intel_display_power_domain power_domain;
+
+ power_domain = intel_display_port_power_domain(encoder);
+ intel_display_power_put(dev_priv, power_domain);
+}
+
static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_i915_private *dev_priv = dev->dev_private;
- enum intel_display_power_domain power_domain;
u32 pp;
u32 pp_stat_reg, pp_ctrl_reg;
bool need_to_disable = !intel_dp->want_panel_vdd;
@@ -1082,8 +1099,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
if (edp_have_panel_vdd(intel_dp))
return need_to_disable;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_dp_power_get(intel_encoder);
DRM_DEBUG_KMS("Turning eDP VDD on\n");
@@ -1133,7 +1149,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
- enum intel_display_power_domain power_domain;
DRM_DEBUG_KMS("Turning eDP VDD off\n");
@@ -1153,8 +1168,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
if ((pp & POWER_TARGET_ON) == 0)
intel_dp->last_power_cycle = jiffies;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_dp_power_put(intel_encoder);
}
}
@@ -1242,7 +1256,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private;
- enum intel_display_power_domain power_domain;
u32 pp;
u32 pp_ctrl_reg;
@@ -1272,8 +1285,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_dp_power_put(intel_encoder);
}
void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -3161,13 +3173,10 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
struct drm_device *dev = connector->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
enum drm_connector_status status;
- enum intel_display_power_domain power_domain;
struct edid *edid = NULL;
intel_runtime_pm_get(dev_priv);
-
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_dp_power_get(intel_encoder);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, drm_get_connector_name(connector));
@@ -3199,8 +3208,7 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
status = connector_status_connected;
out:
- intel_display_power_put(dev_priv, power_domain);
-
+ intel_dp_power_put(intel_encoder);
intel_runtime_pm_put(dev_priv);
return status;
@@ -3213,18 +3221,14 @@ static int intel_dp_get_modes(struct drm_connector
*connector)
struct intel_
[Intel-gfx] [PATCH 4/4] drm/i915: remove useless runtime PM get call
From: Paulo Zanoni
We already call intel_dp_power_get, which will get a power domain, and
every power domain should get a runtime PM reference, which will wake
up the machine.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_dp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a25f708..a0b1cda 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3171,11 +3171,9 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = connector->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
enum drm_connector_status status;
struct edid *edid = NULL;
- intel_runtime_pm_get(dev_priv);
intel_dp_power_get(intel_encoder);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
@@ -3209,7 +3207,6 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
out:
intel_dp_power_put(intel_encoder);
- intel_runtime_pm_put(dev_priv);
return status;
}
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: get a runtime PM ref for the deferred GPU reset work
Atm we can end up in the GPU reset deferred work in D3 state if the last
runtime PM reference is dropped between detecting a hang/scheduling the
work and executing the work. At least one such case I could trigger is
the simulated reset via the i915_wedged debugfs entry. Fix this by
getting an RPM reference around accessing the HW in the reset work.
v2:
- Instead of getting/putting the RPM reference in the reset work itself,
get it already before scheduling the work. By this we also prevent
going to D3 before the work gets to run, in addition to making sure
that we run the work itself in D0. (Ville, Daniel)
v3:
- fix inverted logic fail when putting the RPM ref on behalf of a
cancelled GPU reset work (Ville)
v4:
- Taking the RPM ref in the interrupt handler isn't really needed b/c
it's already guaranteed that we hold an RPM ref until the end of the
reset work in all cases we care about. So take the ref in the reset
work (for cases like i915_wedged_set). (Daniel)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a651d0d..0e47111 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2175,6 +2175,14 @@ static void i915_error_work_func(struct work_struct
*work)
reset_event);
/*
+* In most cases it's guaranteed that we get here with an RPM
+* reference held, for example because there is a pending GPU
+* request that won't finish until the reset is done. This
+* isn't the case at least when we get here by doing a
+* simulated reset via debugs, so get an RPM reference.
+*/
+ intel_runtime_pm_get(dev_priv);
+ /*
* All state reset _must_ be completed before we update the
* reset counter, for otherwise waiters might miss the reset
* pending state and not properly drop locks, resulting in
@@ -2184,6 +2192,8 @@ static void i915_error_work_func(struct work_struct *work)
intel_display_handle_reset(dev);
+ intel_runtime_pm_put(dev_priv);
+
if (ret == 0) {
/*
* After all the gem state is reset, increment the reset
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: get a runtime PM ref for the deferred GPU reset work
Atm we can end up in the GPU reset deferred work in D3 state if the last
runtime PM reference is dropped between detecting a hang/scheduling the
work and executing the work. At least one such case I could trigger is
the simulated reset via the i915_wedged debugfs entry. Fix this by
getting an RPM reference around accessing the HW in the reset work.
v2:
- Instead of getting/putting the RPM reference in the reset work itself,
get it already before scheduling the work. By this we also prevent
going to D3 before the work gets to run, in addition to making sure
that we run the work itself in D0. (Ville, Daniel)
v3:
- fix inverted logic fail when putting the RPM ref on behalf of a
cancelled GPU reset work (Ville)
v4:
- Taking the RPM ref in the interrupt handler isn't really needed b/c
it's already guaranteed that we hold an RPM ref until the end of the
reset work in all cases we care about. So take the ref in the reset
work (for cases like i915_wedged_set). (Daniel)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a651d0d..0e47111 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2175,6 +2175,14 @@ static void i915_error_work_func(struct work_struct
*work)
reset_event);
/*
+* In most cases it's guaranteed that we get here with an RPM
+* reference held, for example because there is a pending GPU
+* request that won't finish until the reset is done. This
+* isn't the case at least when we get here by doing a
+* simulated reset via debugs, so get an RPM reference.
+*/
+ intel_runtime_pm_get(dev_priv);
+ /*
* All state reset _must_ be completed before we update the
* reset counter, for otherwise waiters might miss the reset
* pending state and not properly drop locks, resulting in
@@ -2184,6 +2192,8 @@ static void i915_error_work_func(struct work_struct *work)
intel_display_handle_reset(dev);
+ intel_runtime_pm_put(dev_priv);
+
if (ret == 0) {
/*
* After all the gem state is reset, increment the reset
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: fix infinite loop at gen6_update_ring_freq"
2014-04-11 6:02 GMT-03:00 Daniel Vetter : > On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote: >> On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote: >> > On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote: >> > > This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710. >> > > >> > > This patch duct-tapes over some issue in the current bdw rps patches >> > > which must wait with enabling rc6/rps until the very first batch has >> > > been submitted by userspace. >> > > >> > > But those patches aren't merged yet, and for upstream we need to have >> > > an in-kernel emission of the very first batch. I shouldn't have >> > > merged this patch so let's revert it again. >> > >> > I said this on the mailing last before you merged the patch. >> >> 20140402050338.gb13...@bwidawsk.net > > 20140402145813.GV7225@phenom.ffwll.local will explain things. There's now a regression report pointing to the revert: https://bugs.freedesktop.org/show_bug.cgi?id=77565 . What is the proposed solution now? Just WARN and still avoid the infinite loop? Or keep the infinite loop and leave the bug open? Disable BDW runtime PM? Thanks, Paulo > > -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 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
On Tue, Apr 22, 2014 at 10:17:50PM +0200, Daniel Vetter wrote:
> Otherwise we'll end up spamming dmesg on every context creation on snb
> with vt-d enabled. This regression was introduced in
>
> commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
> Author: Ben Widawsky
> Date: Fri Dec 6 14:11:14 2013 -0800
>
> drm/i915: Reorganize intel_enable_ppgtt
I started to consider what would happen if i915.enable_ppgtt changed on
the fly, but then saw that it is 0400 and this pre-initialisation makes
a lot of sense. So maybe we could mention that here:
As the i915.enable_ppgtt is read-only it cannot be changed after the
module is loaded and so we can perform an early sanitization of the
values.
> References: https://lkml.org/lkml/2014/4/17/599
> Cc: Alessandro Suardi
> Cc: Ben Widawsky
> Signed-off-by: Daniel Vetter
If you care to add in some of the comments,
Reviewed-by: Chris Wilson
> drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0d514ff9b94c..47491c4a1181 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct
> drm_i915_private *dev_priv);
>
> bool intel_enable_ppgtt(struct drm_device *dev, bool full)
> {
> - if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
> + if (i915.enable_ppgtt == 0)
> return false;
>
> if (i915.enable_ppgtt == 1 && full)
> return false;
>
> + return true;
> +}
> +
/* i915.enable_ppgtt is read-only, so do an early pass to validate
* the user's requested state against the hardware/driver capabilities.
* We do this now so that we can print out any log messages once rather
* than every time we check intel_enable_ppgtt().
*/
> +static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> +{
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
v12
Reported-by: Knut Petersen
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson
Cc: Jesse Barnes
---
drivers/gpu/drm/i915/intel_fbdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..28220ca838df 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(&dev->struct_mutex);
+ if (intel_fb &&
+ (sizes->fb_width > intel_fb->base.width ||
+sizes->fb_height > intel_fb->base.height)) {
+ drm_framebuffer_reference(&ifbdev->fb->base);
+ intel_fb = ifbdev->fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb->obj)) {
DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
ret = intelfb_alloc(helper, sizes);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: get a runtime PM ref for the deferred GPU reset work
On Tue, Apr 22, 2014 at 11:34:01PM +0300, Imre Deak wrote: > On Tue, 2014-04-22 at 21:38 +0200, Daniel Vetter wrote: > > On Fri, Apr 18, 2014 at 03:47:45PM +0300, Imre Deak wrote: > > > Atm we can end up in the GPU reset deferred work in D3 state if the last > > > runtime PM reference is dropped between detecting a hang/scheduling the > > > work and executing the work. At least one such case I could trigger is > > > the simulated reset via the i915_wedged debugfs entry. Fix this by > > > disabling RPM before scheduling the work until the end of the work. > > > > > > v2: > > > - Instead of getting/putting the RPM reference in the reset work itself, > > > get it already before scheduling the work. By this we also prevent > > > going to D3 before the work gets to run, in addition to making sure > > > that we run the work itself in D0. (Ville, Daniel) > > > v3: > > > - fix inverted logic fail when putting the RPM ref on behalf of a > > > cancelled GPU reset work (Ville) > > > > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 8 +++- > > > drivers/gpu/drm/i915/i915_irq.c | 21 - > > > 2 files changed, 27 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > > b/drivers/gpu/drm/i915/i915_dma.c > > > index 0b38f88..f792576 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -1823,7 +1823,13 @@ int i915_driver_unload(struct drm_device *dev) > > > > > > /* Free error state after interrupts are fully disabled. */ > > > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > > > - cancel_work_sync(&dev_priv->gpu_error.work); > > > + if (cancel_work_sync(&dev_priv->gpu_error.work)) > > > + /* > > > + * The following won't make any difference in the PM state, > > > + * since RPM is disabled already, but do it still for > > > + * consistency. > > > + */ > > > + intel_runtime_pm_put(dev_priv); > > > i915_destroy_error_state(dev); > > > > > > if (dev->pdev->msi_enabled) > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > b/drivers/gpu/drm/i915/i915_irq.c > > > index a651d0d..5e079d8 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -2210,6 +2210,9 @@ static void i915_error_work_func(struct work_struct > > > *work) > > >*/ > > > i915_error_wake_up(dev_priv, true); > > > } > > > + > > > + /* Drop the ref we took when scheduling this work. */ > > > + intel_runtime_pm_put(dev_priv); > > > } > > > > > > static void i915_report_and_clear_eir(struct drm_device *dev) > > > @@ -2353,8 +2356,24 @@ void i915_handle_error(struct drm_device *dev, > > > bool wedged, > > >* state of outstanding pagelips). Hence it must not be run on our own > > >* dev-priv->wq work queue for otherwise the flush_work in the pageflip > > >* code will deadlock. > > > + * > > > + * It's guaranteed that here we are in D0 state, since we can only get > > > + * here via one of the following paths: > > > + * - From an IRQ handler's error detection. -> The driver must make > > > + * sure that IRQs are unmasked only while holding an RPM ref. > > > + * - From hang-check due to a blocked request. -> The request holds an > > > + * RPM ref, that's only released in i915_gpu_idle() which in turn > > > + * won't be called until the request is finished. > > > + * - From hang-check due to a flip hang. -> We have an RPM ref because > > > + * of the active modeset. > > > + * - From debugfs i915_wedged_set(). -> The caller takes an explicit > > > + * RPM ref. > > > + * Take here an atomic RPM ref still to make sure that we don't > > > + * re-enter D3 until the error work gets to run and completes the > > > + * reset. > > >*/ > > > - schedule_work(&dev_priv->gpu_error.work); > > > + if (schedule_work(&dev_priv->gpu_error.work)) > > > + intel_runtime_pm_get_noresume(dev_priv); > > > > Isn't this a bit racy? A 2nd gpu could immediately execute the work while > > this one here gets interrupted by NMI ... > > It is, I didn't think about that case.. > > > I think we need to grab the ref earlier before the schedule_work. Or we > > just grab it in the reset work around the register access. Actually I > > think this is the right fix since we can only hit this cause by > > fake-injecting a hang through debugfs. For real hangs we'll be holding a > > runtime pm ref until the last request is completed by the gpu, which won't > > happen if the gpu is hung. > > > > Doing the get/put in the worker only also avoids the need to do a get from > > atomic context, which is fairly fragile. > > It's only an atomic inc.. But in case we get here through the interrupt > error detection path, there is no guarantee we have any pending GPU > work, so we could drop the last reference before the work is run. The interrupt based error handling i
Re: [Intel-gfx] [PATCH 2/2] drm/i915: use lane count and link rate from VBT as minimums for eDP
On Tue, Apr 22, 2014 at 08:17:52PM +0300, Jani Nikula wrote:
> Most likely the minimums for both should be enough for enabling the
> native resolution on the eDP.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73539
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76711
> Signed-off-by: Jani Nikula
Tested-by: Markus Blank-Burian
> ---
> drivers/gpu/drm/i915/intel_dp.c | 23 ---
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 80e5598d66ed..c613da9cb1a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -764,8 +764,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc *intel_crtc = encoder->new_crtc;
> struct intel_connector *intel_connector = intel_dp->attached_connector;
> int lane_count, clock;
> + int min_lane_count = 1;
> int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> /* Conveniently, the link BW constants become indices with a shift...*/
> + int min_clock = 0;
> int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
> int bpp, mode_rate;
> static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
> @@ -798,19 +800,26 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> /* Walk through all bpp values. Luckily they're all nicely spaced with 2
>* bpc in between. */
> bpp = pipe_config->pipe_bpp;
> - if (is_edp(intel_dp) && dev_priv->vbt.edp_bpp &&
> - dev_priv->vbt.edp_bpp < bpp) {
> - DRM_DEBUG_KMS("clamping bpp for eDP panel to BIOS-provided
> %i\n",
> - dev_priv->vbt.edp_bpp);
> - bpp = dev_priv->vbt.edp_bpp;
> + if (is_edp(intel_dp)) {
> + if (dev_priv->vbt.edp_bpp && dev_priv->vbt.edp_bpp < bpp) {
> + DRM_DEBUG_KMS("clamping bpp for eDP panel to
> BIOS-provided %i\n",
> + dev_priv->vbt.edp_bpp);
> + bpp = dev_priv->vbt.edp_bpp;
> + }
> +
> + if (dev_priv->vbt.edp_lanes)
> + min_lane_count = min(dev_priv->vbt.edp_lanes,
> + max_lane_count);
> + if (dev_priv->vbt.edp_rate)
> + min_clock = min(dev_priv->vbt.edp_rate >> 3, max_clock);
> }
>
> for (; bpp >= 6*3; bpp -= 2*3) {
> mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> bpp);
>
> - for (lane_count = 1; lane_count <= max_lane_count; lane_count
> <<= 1) {
> - for (clock = 0; clock <= max_clock; clock++) {
> + for (lane_count = min_lane_count; lane_count <= max_lane_count;
> lane_count <<= 1) {
> + for (clock = min_clock; clock <= max_clock; clock++) {
> link_clock =
> drm_dp_bw_code_to_link_rate(bws[clock]);
> link_avail = intel_dp_max_data_rate(link_clock,
> lane_count);
> --
> 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] [RFC 0/3] render state initialization (bdw rc6)
On Tue, Apr 22, 2014 at 08:19:41PM +0300, Mika Kuoppala wrote: > Hi, > > Here are patches to initialize first render context to a hopefully, > valid state. If pipeline/context is not initialized and we enter rc6 on bdw, > the render ring can hung on the first batch submitted. That is atleast > the hypothesis this work is based on. > > The states are stripped from rendercopy_genX's from i-g-t/lib and > the state generators are part of i-g-t also. The states are > propably overshoot from what can be consider the minimal valid > (null) state on the pipeline. I just initialized everything rendercopy > does and haven't really put effort on optimizing the state until > I get some test results that this indeed solves anything. > > The state generators can be found here but they are not needed for testing. > http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen > > Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally > untested. > > Here is the branch for testing: > http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state > > I am interested to know if these patches make matters better/worse for those > who have issues with first batch hanging on bdw, but as always, any feedback > is greatly appreciated. > > Mika Kuoppala (3): > drm/i915: export vmap_batch from command parser It's only a single page, it does not need to be vmapped. > drm/i915: add render state initialization > drm/i915: add null render states for gen6, gen7 and gen8 I still don't buy that this is anything more than papering over a problem. The state you load into the context is invalid as soon as it is executed, which may lead to problems, we don't know since the problem is not being discussed, and it will certainly be more explicit if the right bits are poked into the context directly to keep the hw from falling over. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: get a runtime PM ref for the deferred GPU reset work
On Tue, 2014-04-22 at 21:38 +0200, Daniel Vetter wrote: > On Fri, Apr 18, 2014 at 03:47:45PM +0300, Imre Deak wrote: > > Atm we can end up in the GPU reset deferred work in D3 state if the last > > runtime PM reference is dropped between detecting a hang/scheduling the > > work and executing the work. At least one such case I could trigger is > > the simulated reset via the i915_wedged debugfs entry. Fix this by > > disabling RPM before scheduling the work until the end of the work. > > > > v2: > > - Instead of getting/putting the RPM reference in the reset work itself, > > get it already before scheduling the work. By this we also prevent > > going to D3 before the work gets to run, in addition to making sure > > that we run the work itself in D0. (Ville, Daniel) > > v3: > > - fix inverted logic fail when putting the RPM ref on behalf of a > > cancelled GPU reset work (Ville) > > > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_dma.c | 8 +++- > > drivers/gpu/drm/i915/i915_irq.c | 21 - > > 2 files changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > b/drivers/gpu/drm/i915/i915_dma.c > > index 0b38f88..f792576 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1823,7 +1823,13 @@ int i915_driver_unload(struct drm_device *dev) > > > > /* Free error state after interrupts are fully disabled. */ > > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > > - cancel_work_sync(&dev_priv->gpu_error.work); > > + if (cancel_work_sync(&dev_priv->gpu_error.work)) > > + /* > > +* The following won't make any difference in the PM state, > > +* since RPM is disabled already, but do it still for > > +* consistency. > > +*/ > > + intel_runtime_pm_put(dev_priv); > > i915_destroy_error_state(dev); > > > > if (dev->pdev->msi_enabled) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index a651d0d..5e079d8 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2210,6 +2210,9 @@ static void i915_error_work_func(struct work_struct > > *work) > > */ > > i915_error_wake_up(dev_priv, true); > > } > > + > > + /* Drop the ref we took when scheduling this work. */ > > + intel_runtime_pm_put(dev_priv); > > } > > > > static void i915_report_and_clear_eir(struct drm_device *dev) > > @@ -2353,8 +2356,24 @@ void i915_handle_error(struct drm_device *dev, bool > > wedged, > > * state of outstanding pagelips). Hence it must not be run on our own > > * dev-priv->wq work queue for otherwise the flush_work in the pageflip > > * code will deadlock. > > +* > > +* It's guaranteed that here we are in D0 state, since we can only get > > +* here via one of the following paths: > > +* - From an IRQ handler's error detection. -> The driver must make > > +* sure that IRQs are unmasked only while holding an RPM ref. > > +* - From hang-check due to a blocked request. -> The request holds an > > +* RPM ref, that's only released in i915_gpu_idle() which in turn > > +* won't be called until the request is finished. > > +* - From hang-check due to a flip hang. -> We have an RPM ref because > > +* of the active modeset. > > +* - From debugfs i915_wedged_set(). -> The caller takes an explicit > > +* RPM ref. > > +* Take here an atomic RPM ref still to make sure that we don't > > +* re-enter D3 until the error work gets to run and completes the > > +* reset. > > */ > > - schedule_work(&dev_priv->gpu_error.work); > > + if (schedule_work(&dev_priv->gpu_error.work)) > > + intel_runtime_pm_get_noresume(dev_priv); > > Isn't this a bit racy? A 2nd gpu could immediately execute the work while > this one here gets interrupted by NMI ... It is, I didn't think about that case.. > I think we need to grab the ref earlier before the schedule_work. Or we > just grab it in the reset work around the register access. Actually I > think this is the right fix since we can only hit this cause by > fake-injecting a hang through debugfs. For real hangs we'll be holding a > runtime pm ref until the last request is completed by the gpu, which won't > happen if the gpu is hung. > > Doing the get/put in the worker only also avoids the need to do a get from > atomic context, which is fairly fragile. It's only an atomic inc.. But in case we get here through the interrupt error detection path, there is no guarantee we have any pending GPU work, so we could drop the last reference before the work is run. > We should have a comment in the reset work explaining why we need it > though. > -Daniel > > > } > > > > static void __always_unused i915_pageflip_stall_check(struct drm_device > > *dev, int pipe) > > -- > > 1
Re: [Intel-gfx] [PATCH 2/3] drm/i915: add render state initialization
On Tue, Apr 22, 2014 at 08:19:43PM +0300, Mika Kuoppala wrote:
> HW guys say that it is not a cool idea to let device
> go into rc6 without proper 3d pipeline state.
>
> For each new uninitialized context, generate a
> valid null render state to be run on context
> creation.
>
> This patch introduces a skeleton with emty states.
>
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/Makefile |1 +
> drivers/gpu/drm/i915/i915_drv.h |2 +
> drivers/gpu/drm/i915/i915_gem_context.c |7 +
> drivers/gpu/drm/i915/i915_gem_render_state.c | 222
> +
> drivers/gpu/drm/i915/intel_renderstate_gen6.h | 11 ++
> drivers/gpu/drm/i915/intel_renderstate_gen7.h | 11 ++
> drivers/gpu/drm/i915/intel_renderstate_gen8.h | 11 ++
Imo static const arrays in headers look ugly ... I prefer we just add
normal C files with forward declarations in i915_gem_render_state.c.
In the i915/Makefile you could add a new section for all these golden
renderstate files. I know this is a bikeshed, but ;-)
-Daniel
> 7 files changed, 265 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
> create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
> create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
> create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index b1445b7..b5d4029 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
> # GEM code
> i915-y += i915_cmd_parser.o \
> i915_gem_context.o \
> + i915_gem_render_state.o \
> i915_gem_debug.o \
> i915_gem_dmabuf.o \
> i915_gem_evict.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43b022c..f6ae2ee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device
> *dev, void *data,
> int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
>
> +/* i915_gem_render_state.c */
> +int i915_gem_init_render_state(struct intel_ring_buffer *ring);
> /* i915_gem_evict.c */
> int __must_check i915_gem_evict_something(struct drm_device *dev,
> struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 30b355a..d648d4d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
> /* obj is kept alive until the next request by its active ref */
> i915_gem_object_ggtt_unpin(from->obj);
> i915_gem_context_unreference(from);
> + } else {
> + if (ring->id == RCS && to->is_initialized == false) {
> +
> + ret = i915_gem_init_render_state(ring);
> + if (ret)
> + DRM_ERROR("init render state: %d\n", ret);
> + }
> }
>
> to->is_initialized = true;
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
> b/drivers/gpu/drm/i915/i915_gem_render_state.c
> new file mode 100644
> index 000..409f99b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -0,0 +1,222 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Mika Kuoppala
> + *
> + */
> +
> +#include "i915_drv.h"
> +
> +#include "intel_renderstate_gen6.h"
> +#include "intel_renderstate_gen7.h"
> +#
Re: [Intel-gfx] [PATCH 00/10] Enable RC6/Turbo on CHV
On Mon, Apr 21, 2014 at 07:28:54PM +0530, Deepak S wrote: > Hi Ville, > > let me know if you want some of other small patches to be squashed. Quick aside: Something seems to have gone with git send-email thread - the patches aren't in-reply-to the cover letter. No idea how that happened though ... -Daniel > > Thanks > Deepak > > > On Monday 21 April 2014 01:23 PM, deepa...@linux.intel.com wrote: > >From: Deepak S > > > >Squashed some of the patches and created a new patch series. > > > >ToDo: Address the comments on some the patches. Changes will be shared in > >next series. > > > >Ben Widawsky (1): > > drm/i915/bdw: Implement a basic PM interrupt handler > > > >Deepak S (6): > > drm/i915: Enable PM Interrupts target via Display Interface. > > drm/i915/chv: Enable Render Standby (RC6) for Cheeryview > > drm/i915/chv: Added CHV specific register read and write > > drm/i915/chv: Enable RPS (Turbo) for Cheeryview > > drm/i915/chv: Added CHV specific DDR fetch into init_clock_gating > > drm/i915/chv: Freq(opcode) request value for CHV. > > > >Ville Syrjälä (3): > > drm/i915/chv: Streamline CHV forcewake stuff > > drm/i915/chv: CHV doesn't need WaRsForcewakeWaitTC0 > > drm/i915/chv: Skip gen6_gt_check_fifodbg() on CHV > > > > drivers/gpu/drm/i915/i915_drv.h | 10 ++ > > drivers/gpu/drm/i915/i915_irq.c | 79 +++- > > drivers/gpu/drm/i915/i915_reg.h | 13 ++ > > drivers/gpu/drm/i915/intel_drv.h | 2 + > > drivers/gpu/drm/i915/intel_pm.c | 231 > > +- > > drivers/gpu/drm/i915/intel_sideband.c | 15 +++ > > drivers/gpu/drm/i915/intel_uncore.c | 126 +-- > > 7 files changed, 455 insertions(+), 21 deletions(-) > > > > ___ > 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
[Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi
Cc: Ben Widawsky
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..47491c4a1181 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct drm_i915_private
*dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 && full)
return false;
+ return true;
+}
+
+static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+{
+ if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ return 0;
+
+ if (i915.enable_ppgtt == 1)
+ return 1;
+
+ if (i915.enable_ppgtt == 2 && HAS_PPGTT(dev))
+ return 2;
+
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
DRM_INFO("Disabling PPGTT because VT-d is on\n");
- return false;
+ return 0;
}
#endif
- /* Full ppgtt disabled by default for now due to issues. */
- if (full)
- return HAS_PPGTT(dev) && (i915.enable_ppgtt == 2);
- else
- return HAS_ALIASING_PPGTT(dev);
+ return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
}
@@ -1157,6 +1167,8 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
struct drm_i915_private *dev_priv = dev->dev_private;
int ret = 0;
+ i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
+
ppgtt->base.dev = dev;
ppgtt->base.scratch = dev_priv->gtt.base.scratch;
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Fwd: 3.15-rc1: Endless "Disabling PPGTT because VT-d is on" messages in /var/log/messages
On Fri, Apr 18, 2014 at 06:29:45PM +0200, Alessandro Suardi wrote: > forwarding from my lkml post as probably more appropriate to send this here... Yup, but just adding more cc's is preferred. I've re-added lkml for reference. Bug is fairly obvious regression in 3.15, patch is on the way. -Daniel > > thanks, > > -- Forwarded message -- > From: Alessandro Suardi > Date: Fri, Apr 18, 2014 at 12:31 AM > Subject: 3.15-rc1: Endless "Disabling PPGTT because VT-d is on" > messages in /var/log/messages > To: "linux-ker...@vger.kernel.org" > > > [hoping I persuaded gmail to get my old text-only email settings back, grrr] > > > Noticed that my root fs was filling up, and found out that /var/log/messages > was > approaching 2GB of space, most of which were as per $subject. To give an idea > of the magnitude, here's a snippet from a saved sample of /var/log/messages: > > [root@xbox log]# grep "Disabling PP" messages.save.head |head -1 > Apr 15 03:44:10 xbox kernel: [3.115126] [drm] Disabling PPGTT > because VT-d is on > [root@xbox log]# grep "Disabling PP" messages.save.head |tail -1 > Apr 15 03:45:50 xbox kernel: [ 130.165219] [drm] Disabling PPGTT > because VT-d is on > [root@xbox log]# grep -c "Disabling PP" messages.save.head > 610 > > Or from the dmesg ring: > > [root@xbox log]# dmesg|head > [22446.690572] [drm] Disabling PPGTT because VT-d is on > [22446.690616] [drm] Disabling PPGTT because VT-d is on > [22446.704560] [drm] Disabling PPGTT because VT-d is on > [22446.706747] [drm] Disabling PPGTT because VT-d is on > [22446.720202] [drm] Disabling PPGTT because VT-d is on > [22446.722447] [drm] Disabling PPGTT because VT-d is on > [22446.724850] [drm] Disabling PPGTT because VT-d is on > [22446.727268] [drm] Disabling PPGTT because VT-d is on > [22446.733052] [drm] Disabling PPGTT because VT-d is on > [22446.735319] [drm] Disabling PPGTT because VT-d is on > > > > Are there patches for this issue, or is a workaround available to prevent a > filesystem full condition on my root fs? > > > This is on a Dell Latitude E6420 laptop, Fedora 19 x86_64, onboard > Intel video card... yes, I have an old A08 BIOS, but I've never seen > this issue while regularly updating my mainline kernel weekly or so... > > > Available for more details if needed, thanks in advance, > > --alessandro > > "don't underestimate the things that I will do" > > (Adele, "Rolling In The Deep") > > > -- > --alessandro > > "don't underestimate the things that I will do" > > (Adele, "Rolling In The Deep") > ___ > 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: Make the physical object coherent with GTT
On Fri, Apr 18, 2014 at 08:27:04AM +0100, Chris Wilson wrote:
> Currently objects for which the hardware needs a contiguous physical
> address are allocated a shadow backing storage to satisfy the contraint.
> This shadow buffer is not wired into the normal obj->pages and so the
> physical object is incoherent with accesses via the GPU, GTT and CPU. By
> setting up the appropriate scatter-gather table, we can allow userspace
> to access the physical object via either a GTT mmaping of or by rendering
> into the GEM bo. However, keeping the CPU mmap of the shmemfs backing
> storage coherent with the contiguous shadow is not yet possible.
> Fortuituously, CPU mmaps of objects requiring physical addresses are not
> expected to be coherent anyway.
>
> This allows the physical constraint of the GEM object to be transparent
> to userspace and allow it to efficiently render into or update them via
> the GTT and GPU.
>
> Signed-off-by: Chris Wilson
You know the drill: I'd like to have a small igt for this which creates a
cursor bo, puts it to use and then checks whether the pwrite -> gtt mmap
read path is coherent.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_dma.c | 3 +
> drivers/gpu/drm/i915/i915_gem.c | 182
> +++-
> include/uapi/drm/i915_drm.h | 1 +
> 3 files changed, 129 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 12571aac7516..dd43fdc736ac 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1022,6 +1022,9 @@ static int i915_getparam(struct drm_device *dev, void
> *data,
> case I915_PARAM_CMD_PARSER_VERSION:
> value = i915_cmd_parser_get_version();
> break;
> + case I915_PARAM_HAS_COHERENT_PHYS_GTT:
> + value = 1;
> + break;
> default:
> DRM_DEBUG("Unknown parameter %d\n", param->param);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e302a23031fe..3eb5c07e1018 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -207,41 +207,129 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev,
> void *data,
> return 0;
> }
>
> -static void i915_gem_object_detach_phys(struct drm_i915_gem_object *obj)
> +static int
> +i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
> {
> - drm_dma_handle_t *phys = obj->phys_handle;
> + struct address_space *mapping = file_inode(obj->base.filp)->i_mapping;
> + char *vaddr = obj->phys_handle->vaddr;
> + struct sg_table *st;
> + struct scatterlist *sg;
> + int i;
>
> - if (!phys)
> - return;
> + if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> + return -EINVAL;
> +
> + for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> + struct page *page;
> + char *src;
> +
> + page = shmem_read_mapping_page(mapping, i);
> + if (IS_ERR(page))
> + return PTR_ERR(page);
> +
> + src = kmap_atomic(page);
> + memcpy(vaddr, src, PAGE_SIZE);
> + kunmap_atomic(src);
> +
> + page_cache_release(page);
> + vaddr += PAGE_SIZE;
> + }
> +
> + drm_clflush_virt_range(obj->phys_handle->vaddr, obj->base.size);
> + i915_gem_chipset_flush(obj->base.dev);
> +
> + st = kmalloc(sizeof(*st), GFP_KERNEL);
> + if (st == NULL)
> + return -ENOMEM;
> +
> + if (sg_alloc_table(st, 1, GFP_KERNEL)) {
> + kfree(st);
> + return -ENOMEM;
> + }
> +
> + sg = st->sgl;
> + sg->offset = 0;
> + sg->length = obj->base.size;
> +
> + sg_dma_address(sg) = obj->phys_handle->busaddr;
> + sg_dma_len(sg) = obj->base.size;
>
> - if (obj->madv == I915_MADV_WILLNEED) {
> + obj->pages = st;
> + obj->has_dma_mapping = true;
> + return 0;
> +}
> +
> +static void
> +i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj)
> +{
> + int ret;
> +
> + BUG_ON(obj->madv == __I915_MADV_PURGED);
> +
> + ret = i915_gem_object_set_to_cpu_domain(obj, true);
> + if (ret) {
> + /* In the event of a disaster, abandon all caches and
> + * hope for the best.
> + */
> + WARN_ON(ret != -EIO);
> + obj->base.read_domains = obj->base.write_domain =
> I915_GEM_DOMAIN_CPU;
> + }
> +
> + if (obj->madv == I915_MADV_DONTNEED)
> + obj->dirty = 0;
> +
> + if (obj->dirty) {
> struct address_space *mapping =
> file_inode(obj->base.filp)->i_mapping;
> - char *vaddr = phys->vaddr;
> + char *vaddr = obj->phys_handle->vaddr;
> int i;
>
> for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> - struct page *page = shmem_rea
Re: [Intel-gfx] [PATCH V4 3/6] drm/i915:Initialize the second BSD ring on BDW GT3 machine
On Thu, Apr 17, 2014 at 10:37:37AM +0800, Zhao Yakui wrote:
> Based on the hardware spec, the BDW GT3 machine has two independent
> BSD ring that can be used to dispatch the video commands.
> So just initialize it.
>
> V3->V4: Follow Imre's comment to do some minor updates. For example:
> more comments are added to describe the semaphore between ring.
Within a patch series we usually keep revisions for each patch separately,
so this would only be v2 for this patch.
Once a patch is merge people won't ever look at it in context of your
entire series, but just as an individual patch. If your in-patch commit
log directly jumps to v4 from v1 then people are left wondering what
happened to v2 and v3 ;-)
Anyway just a small nit for the next patch series.
-Daniel
>
> Reviewed-by: Imre Deak
> Signed-off-by: Zhao Yakui
> ---
> drivers/gpu/drm/i915/i915_drv.c |4 +-
> drivers/gpu/drm/i915/i915_drv.h |2 +
> drivers/gpu/drm/i915/i915_gem.c |9 +++-
> drivers/gpu/drm/i915/i915_gpu_error.c |1 +
> drivers/gpu/drm/i915/i915_reg.h |1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 78
> +++
> drivers/gpu/drm/i915/intel_ringbuffer.h |4 +-
> 7 files changed, 95 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 17fbbe5..2a7842b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -282,7 +282,7 @@ static const struct intel_device_info
> intel_broadwell_m_info = {
> static const struct intel_device_info intel_broadwell_gt3d_info = {
> .gen = 8, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> + .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> .has_llc = 1,
> .has_ddi = 1,
> .has_fbc = 1,
> @@ -292,7 +292,7 @@ static const struct intel_device_info
> intel_broadwell_gt3d_info = {
> static const struct intel_device_info intel_broadwell_gt3m_info = {
> .gen = 8, .is_mobile = 1, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> + .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> .has_llc = 1,
> .has_ddi = 1,
> .has_fbc = 1,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 92c3095..74aef6a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1833,7 +1833,9 @@ struct drm_i915_cmd_table {
> #define BSD_RING (1< #define BLT_RING (1< #define VEBOX_RING (1< +#define BSD2_RING(1< #define HAS_BSD(dev)(INTEL_INFO(dev)->ring_mask & BSD_RING)
> +#define HAS_BSD2(dev)(INTEL_INFO(dev)->ring_mask & BSD2_RING)
> #define HAS_BLT(dev)(INTEL_INFO(dev)->ring_mask & BLT_RING)
> #define HAS_VEBOX(dev)(INTEL_INFO(dev)->ring_mask & VEBOX_RING)
> #define HAS_LLC(dev)(INTEL_INFO(dev)->has_llc)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 85c9cf0..65c441c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4374,13 +4374,20 @@ static int i915_gem_init_rings(struct drm_device *dev)
> goto cleanup_blt_ring;
> }
>
> + if (HAS_BSD2(dev)) {
> + ret = intel_init_bsd2_ring_buffer(dev);
> + if (ret)
> + goto cleanup_vebox_ring;
> + }
>
> ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> if (ret)
> - goto cleanup_vebox_ring;
> + goto cleanup_bsd2_ring;
>
> return 0;
>
> +cleanup_bsd2_ring:
> + intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
> cleanup_vebox_ring:
> intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
> cleanup_blt_ring:
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 4865ade..282164c 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -42,6 +42,7 @@ static const char *ring_str(int ring)
> case VCS: return "bsd";
> case BCS: return "blt";
> case VECS: return "vebox";
> + case VCS2: return "bsd2";
> default: return "";
> }
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f84555..0b88508 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -760,6 +760,7 @@ enum punit_power_well {
> #define RENDER_RING_BASE 0x02000
> #define BSD_RING_BASE0x04000
> #define GEN6_BSD_RING_BASE 0x12000
> +#define GEN8_BSD2_RING_BASE 0x1c000
> #define VEBOX_RING_BASE 0x1a000
> #define BLT_RING_BASE0x22000
Re: [Intel-gfx] [PATCH I-g-t V2 2/2] tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU<->GPU sync under multi BSD rings
On Tue, Apr 22, 2014 at 03:05:03PM +0300, Imre Deak wrote:
> On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> > The Broadwell GT3 machine has two independent BSD rings in kernel driver
> > while
> > it is transparent to the user-space driver. In such case it needs to check
> > the CPU<->GPU sync for the second BSD ring.
> >
> > V1->V2: Follow Daniel's comment to add one subtext instead of one individual
> > test case, which is used to test the CPU<->GPU sync under multi BSD rings
> >
> > Signed-off-by: Zhao Yakui
> > ---
> > tests/gem_dummy_reloc_loop.c | 102
> > +-
> > 1 file changed, 101 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
> > index a61b59b..660d8e1 100644
> > --- a/tests/gem_dummy_reloc_loop.c
> > +++ b/tests/gem_dummy_reloc_loop.c
> > @@ -48,6 +48,13 @@ static drm_intel_bufmgr *bufmgr;
> > struct intel_batchbuffer *batch;
> > static drm_intel_bo *target_buffer;
> >
> > +#define NUM_FD 50
> > +
> > +static int mfd[NUM_FD];
> > +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> > +static struct intel_batchbuffer *mbatch[NUM_FD];
> > +static drm_intel_bo *mbuffer[NUM_FD];
> > +
> > /*
> > * Testcase: Basic check of ring<->cpu sync using a dummy reloc
> > *
> > @@ -124,6 +131,50 @@ dummy_reloc_loop_random_ring(int num_rings)
> > }
> > }
> >
> > +static void
> > +dummy_reloc_loop_random_ring_multi_fd(int num_rings)
> > +{
> > + int i;
> > + struct intel_batchbuffer *saved_batch;
> > +
> > + saved_batch = batch;
> > +
> > + srandom(0xdeadbeef);
> > +
> > + for (i = 0; i < 0x10; i++) {
> > + int mindex;
> > + int ring = random() % num_rings + 1;
> > +
> > + mindex = random() % NUM_FD;
> > + batch = mbatch[mindex];
> > +
> > + if (ring == I915_EXEC_RENDER) {
> > + BEGIN_BATCH(4);
> > + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> > + OUT_BATCH(0x); /* compare dword */
> > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > + I915_GEM_DOMAIN_RENDER, 0);
> > + OUT_BATCH(MI_NOOP);
> > + ADVANCE_BATCH();
> > + } else {
> > + BEGIN_BATCH(4);
> > + OUT_BATCH(MI_FLUSH_DW | 1);
> > + OUT_BATCH(0); /* reserved */
> > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > + I915_GEM_DOMAIN_RENDER, 0);
> > + OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
> > + ADVANCE_BATCH();
> > + }
> > + intel_batchbuffer_flush_on_ring(batch, ring);
> > +
> > + drm_intel_bo_map(target_buffer, 0);
> > + // map to force waiting on rendering
> > + drm_intel_bo_unmap(target_buffer);
> > + }
> > +
> > + batch = saved_batch;
> > +}
> > +
> > int fd;
> > int devid;
> > int num_rings;
> > @@ -133,6 +184,7 @@ igt_main
> > igt_skip_on_simulation();
> >
> > igt_fixture {
> > + int i;
> > fd = drm_open_any();
> > devid = intel_get_drm_devid(fd);
> > num_rings = gem_get_num_rings(fd);
> > @@ -148,6 +200,35 @@ igt_main
> >
> > target_buffer = drm_intel_bo_alloc(bufmgr, "target bo", 4096,
> > 4096);
> > igt_assert(target_buffer);
> > +
> > + /* Create multi drm_fd and map one gem object to multi
> > gem_contexts */
> > + {
> > + unsigned int target_flink;
> > + char buffer_name[32];
> > + if (dri_bo_flink(target_buffer, &target_flink)) {
> > + printf("fail to get flink for target buffer\n");
> > + igt_assert(0);
>
> For the future: could be just igt_assert_f().
Yeah I think for new testcases we should try to use the latest igt_*
macros and helpers as much as possible. Reducing control flow and
replacing it by the right igt_assert/require/... macro imo really helps
the readability of testcases.
-Daniel
>
> > + }
> > + for (i = 0; i < NUM_FD; i++) {
> > + mfd[i] = 0;
> > + mbufmgr[i] = NULL;
> > + mbuffer[i] = NULL;
> > + }
>
> Nitpick: the above are all statics, so no need to init them.
>
> Other than the above this looks good:
> Reviewed-by: Imre Deak
>
> > + for (i = 0; i < NUM_FD; i++) {
> > + sprintf(buffer_name, "Target buffer %d\n", i);
> > + mfd[i] = drm_open_any();
> > + mbufmgr[i] = drm_intel_bufmgr_gem_init(mfd[i],
> > 4096);
> > + igt_assert(mbufmgr[i]);
> > + drm_intel_bufmgr_gem_enable_
Re: [Intel-gfx] [PATCH I-g-t V2 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
On Tue, Apr 22, 2014 at 02:52:04PM +0300, Imre Deak wrote:
> On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> > The Broadwell GT3 machine has two independent BSD rings in kernel driver
> > while
> > it is transparent to the user-space driver. In such case it needs to check
> > the ring sync between the two BSD rings. At the same time it also needs to
> > check the sync among the second BSD ring and the other rings.
> >
> > Signed-off-by: Zhao Yakui
> > ---
> > tests/Makefile.sources |1 +
> > tests/gem_multi_bsd_sync_loop.c | 172
> > +++
> > 2 files changed, 173 insertions(+)
> > create mode 100644 tests/gem_multi_bsd_sync_loop.c
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index c957ace..7cd9ca8 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -105,6 +105,7 @@ TESTS_progs = \
> > gem_render_tiled_blits \
> > gem_ring_sync_copy \
> > gem_ring_sync_loop \
> > + gem_multi_bsd_sync_loop \
> > gem_seqno_wrap \
> > gem_set_tiling_vs_gtt \
> > gem_set_tiling_vs_pwrite \
> > diff --git a/tests/gem_multi_bsd_sync_loop.c
> > b/tests/gem_multi_bsd_sync_loop.c
> > new file mode 100644
> > index 000..7f5b832
> > --- /dev/null
> > +++ b/tests/gem_multi_bsd_sync_loop.c
> > @@ -0,0 +1,172 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > next
> > + * paragraph) shall be included in all copies or substantial portions of
> > the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + * Authors:
> > + *Daniel Vetter (based on
> > gem_ring_sync_loop_*.c)
> > + *Zhao Yakui
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "drm.h"
> > +#include "ioctl_wrappers.h"
> > +#include "drmtest.h"
> > +#include "intel_bufmgr.h"
> > +#include "intel_batchbuffer.h"
> > +#include "intel_io.h"
> > +#include "i830_reg.h"
> > +#include "intel_chipset.h"
> > +
> > +static drm_intel_bufmgr *bufmgr;
> > +struct intel_batchbuffer *batch;
> > +static drm_intel_bo *target_buffer;
> > +
> > +#define NUM_FD 50
> > +
> > +static int mfd[NUM_FD];
> > +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> > +static struct intel_batchbuffer *mbatch[NUM_FD];
> > +static drm_intel_bo *mbuffer[NUM_FD];
> > +
> > +
> > +/*
> > + * Testcase: Basic check of ring<->ring sync using a dummy reloc
> > + *
> > + * Extremely efficient at catching missed irqs with semaphores=0 ...
> > + */
> > +
> > +#define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
> > +#define MI_DO_COMPARE (1<<21)
> > +
> > +static void
> > +store_dword_loop(int fd)
> > +{
> > + int i;
> > + int num_rings = gem_get_num_rings(fd);
> > +
> > + srandom(0xdeadbeef);
> > +
> > + for (i = 0; i < SLOW_QUICK(0x10, 10); i++) {
> > + int ring, mindex;
> > + ring = random() % num_rings + 1;
> > + mindex = random() % NUM_FD;
> > + batch = mbatch[mindex];
> > + if (ring == I915_EXEC_RENDER) {
> > + BEGIN_BATCH(4);
> > + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> > + OUT_BATCH(0x); /* compare dword */
> > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > + I915_GEM_DOMAIN_RENDER, 0);
> > + OUT_BATCH(MI_NOOP);
> > + ADVANCE_BATCH();
> > + } else {
> > + BEGIN_BATCH(4);
> > + OUT_BATCH(MI_FLUSH_DW | 1);
> > + OUT_BATCH(0); /* reserved */
> > + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> > + I915_GEM_DOMAIN_RENDER, 0);
> > + OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
> > +
Re: [Intel-gfx] [PATCH v3] drm/i915: get a runtime PM ref for the deferred GPU reset work
On Fri, Apr 18, 2014 at 03:47:45PM +0300, Imre Deak wrote: > Atm we can end up in the GPU reset deferred work in D3 state if the last > runtime PM reference is dropped between detecting a hang/scheduling the > work and executing the work. At least one such case I could trigger is > the simulated reset via the i915_wedged debugfs entry. Fix this by > disabling RPM before scheduling the work until the end of the work. > > v2: > - Instead of getting/putting the RPM reference in the reset work itself, > get it already before scheduling the work. By this we also prevent > going to D3 before the work gets to run, in addition to making sure > that we run the work itself in D0. (Ville, Daniel) > v3: > - fix inverted logic fail when putting the RPM ref on behalf of a > cancelled GPU reset work (Ville) > > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_dma.c | 8 +++- > drivers/gpu/drm/i915/i915_irq.c | 21 - > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 0b38f88..f792576 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1823,7 +1823,13 @@ int i915_driver_unload(struct drm_device *dev) > > /* Free error state after interrupts are fully disabled. */ > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > - cancel_work_sync(&dev_priv->gpu_error.work); > + if (cancel_work_sync(&dev_priv->gpu_error.work)) > + /* > + * The following won't make any difference in the PM state, > + * since RPM is disabled already, but do it still for > + * consistency. > + */ > + intel_runtime_pm_put(dev_priv); > i915_destroy_error_state(dev); > > if (dev->pdev->msi_enabled) > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index a651d0d..5e079d8 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2210,6 +2210,9 @@ static void i915_error_work_func(struct work_struct > *work) >*/ > i915_error_wake_up(dev_priv, true); > } > + > + /* Drop the ref we took when scheduling this work. */ > + intel_runtime_pm_put(dev_priv); > } > > static void i915_report_and_clear_eir(struct drm_device *dev) > @@ -2353,8 +2356,24 @@ void i915_handle_error(struct drm_device *dev, bool > wedged, >* state of outstanding pagelips). Hence it must not be run on our own >* dev-priv->wq work queue for otherwise the flush_work in the pageflip >* code will deadlock. > + * > + * It's guaranteed that here we are in D0 state, since we can only get > + * here via one of the following paths: > + * - From an IRQ handler's error detection. -> The driver must make > + * sure that IRQs are unmasked only while holding an RPM ref. > + * - From hang-check due to a blocked request. -> The request holds an > + * RPM ref, that's only released in i915_gpu_idle() which in turn > + * won't be called until the request is finished. > + * - From hang-check due to a flip hang. -> We have an RPM ref because > + * of the active modeset. > + * - From debugfs i915_wedged_set(). -> The caller takes an explicit > + * RPM ref. > + * Take here an atomic RPM ref still to make sure that we don't > + * re-enter D3 until the error work gets to run and completes the > + * reset. >*/ > - schedule_work(&dev_priv->gpu_error.work); > + if (schedule_work(&dev_priv->gpu_error.work)) > + intel_runtime_pm_get_noresume(dev_priv); Isn't this a bit racy? A 2nd gpu could immediately execute the work while this one here gets interrupted by NMI ... I think we need to grab the ref earlier before the schedule_work. Or we just grab it in the reset work around the register access. Actually I think this is the right fix since we can only hit this cause by fake-injecting a hang through debugfs. For real hangs we'll be holding a runtime pm ref until the last request is completed by the gpu, which won't happen if the gpu is hung. Doing the get/put in the worker only also avoids the need to do a get from atomic context, which is fairly fragile. We should have a comment in the reset work explaining why we need it though. -Daniel > } > > static void __always_unused i915_pageflip_stall_check(struct drm_device > *dev, int pipe) > -- > 1.8.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] mm: Throttle shrinkers harder
On Fri, Apr 18, 2014 at 12:14:16PM -0700, Andrew Morton wrote: > On Thu, 10 Apr 2014 08:05:06 +0100 Chris Wilson > wrote: > > > During testing of i915.ko with working texture sets larger than RAM, we > > encounter OOM with plenty of memory still trapped within writeback, e.g: > > > > [ 42.386039] active_anon:10134 inactive_anon:1900781 isolated_anon:32 > > active_file:33 inactive_file:39 isolated_file:0 > > unevictable:0 dirty:0 writeback:337627 unstable:0 > > free:11985 slab_reclaimable:9458 slab_unreclaimable:23614 > > mapped:41 shmem:1560769 pagetables:1276 bounce:0 > > > > If we throttle for writeback following shrink_slab, this gives us time > > to wait upon the writeback generated by the i915.ko shinker: > > > > [ 4756.750808] active_anon:24386 inactive_anon:900793 isolated_anon:0 > > active_file:23 inactive_file:20 isolated_file:0 > > unevictable:0 dirty:0 writeback:0 unstable:0 > > free:5550 slab_reclaimable:5184 slab_unreclaimable:4888 > > mapped:3 shmem:472393 pagetables:1249 bounce:0 > > > > (Sadly though the test is still failing.) > > > > Testcase: igt/gem_tiled_swapping > > References: https://bugs.freedesktop.org/show_bug.cgi?id=72742 > > i915_gem_object_get_pages_gtt() makes my head spin, but > https://bugs.freedesktop.org/attachment.cgi?id=90818 says > "gfp_mask=0x201da" which is > > ___GFP_HARDWALL|___GFP_COLD|___GFP_FS|___GFP_IO|___GFP_WAIT|___GFP_MOVABLE|___GFP_HIGHMEM > > so this allocation should work and it very bad if the page allocator is > declaring oom while there is so much writeback in flight, assuming the > writeback is to eligible zones. For more head spinning look at the lock stealing dance we do in our shrinker callbacks i915_gem_inactive_scan|count(). It's not pretty at all, but it helps to avoids the dreaded oom in a few more cases. Some review of our mess of ducttape from -mm developers with actual clue would be really appreciated ... -Daniel > Mel, Johannes: could you take a look please? > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majord...@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: mailto:"d...@kvack.org";> em...@kvack.org -- 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 6/6] drm/i915: Kick start the rings
On Tue, Apr 22, 2014 at 01:06:45PM +, Mateo Lozano, Oscar wrote: > > Subject: [Intel-gfx] [PATCH 6/6] drm/i915: Kick start the rings > > > > On g4x, we have an issue where the register write to setup the rings do not > > always take. However, it appears that the current check also passes only by > > chance, a second reading of the register returns a different broekn value - > > but > > the GPU appears to function. Based on that observation, lets try feeding a > > nop > > into the ring and seeing if it advances as part of our startup sanity > > checks. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index a8c73a0d935d..bc52645fa8d5 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -514,10 +514,14 @@ static int init_ring_common(struct intel_ring_buffer > > *ring) > > ((ring->size - PAGE_SIZE) & RING_NR_PAGES) > > | RING_VALID); > > > > + iowrite32(MI_NOOP, ring->virtual_start + 0); > > + iowrite32(MI_NOOP, ring->virtual_start + 4); > > + ring->write_tail(ring, 8); > > Maybe add a comment in the code about why we are doing this? (otherwise, it > looks a bit magical) Yeah, this is a bit too much magic. Also it looks like it doesn't really help the bug report with his gm45, so I think I'll punt on this one for now. All other patches merged with the 2 small suggestions from Oscar applied while merging. Thanks for patches&review. -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 1/2] drm/i915: remove unexplained vblank wait in the DP off code
On Thu, Apr 17, 2014 at 03:21:27PM +0300, Ville Syrjälä wrote: > On Fri, Apr 11, 2014 at 02:25:41PM -0700, Jesse Barnes wrote: > > I don't think this is necessary; at least it doesn't appear to be on my > > BYT. Dropping it speeds up our shutdown code a little, in some cases > > resulting in faster init times. > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/intel_dp.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index e48d47c..728a5db 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2756,9 +2756,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) > > } > > POSTING_READ(intel_dp->output_reg); > > > > - /* We don't really know why we're doing this */ > > - intel_wait_for_vblank(dev, intel_crtc->pipe); > > - > > Maybe this was here to guarantee we send the magic five idle patterns > specified in the DP spec. But since we're going to be turning off the > port anyway I don't see why we switch to transmitting the idle pattern > at all. > > I guess switching to the idle pattern might make sense for the IBX > transcoder select workaround to avoid sending some garbage on the main > link. Although we don't seem to be doing that workaround quite according > to spec. The spec says we should first disable the port, and then > re-enable it temporarily w/ transcoder A. What we do is switch the port > over to transcoder A while it's still enabled, and only then disable it. > > So I guess killing the wait here is fine, but looks like the IBX > workaround stuff needs a better look. I can try to clean it up a bit. > > Reviewed-by: Ville Syrjälä Queued for -next, thanks for the patch. -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: Adding debug code for dp_m2_n2 in crtc_config
On Fri, Apr 11, 2014 at 05:01:20PM +0530, Vandana Kannan wrote:
> Adding relevant read out comparison code, in check_crtc_state, for the new
> member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
> values for a DP downclock mode (if available). Suggested by Daniel.
>
> Signed-off-by: Vandana Kannan
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_display.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1af1d14..36fc034 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9199,6 +9199,15 @@ static void intel_dump_pipe_config(struct intel_crtc
> *crtc,
> pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
> pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
> pipe_config->dp_m_n.tu);
> +
> + DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2:
> %u, tu2: %u\n",
> + pipe_config->has_dp_encoder,
> + pipe_config->dp_m2_n2.gmch_m,
> + pipe_config->dp_m2_n2.gmch_n,
> + pipe_config->dp_m2_n2.link_m,
> + pipe_config->dp_m2_n2.link_n,
> + pipe_config->dp_m2_n2.tu);
> +
> DRM_DEBUG_KMS("requested mode:\n");
> drm_mode_debug_printmodeline(&pipe_config->requested_mode);
> DRM_DEBUG_KMS("adjusted mode:\n");
> @@ -9619,6 +9628,12 @@ intel_pipe_config_compare(struct drm_device *dev,
> PIPE_CONF_CHECK_I(dp_m_n.link_n);
> PIPE_CONF_CHECK_I(dp_m_n.tu);
>
> + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_m);
> + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_n);
> + PIPE_CONF_CHECK_I(dp_m2_n2.link_m);
> + PIPE_CONF_CHECK_I(dp_m2_n2.link_n);
> + PIPE_CONF_CHECK_I(dp_m2_n2.tu);
> +
> PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
> PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
> PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
Either I'm missing something or your patch lacks state readout support for
pipe_config->dp_m2_n2. It /should/ result in piles of WARNING backtraces
when you run this. Or you've based this on an Android tree which has the
state cross checker disabled. I'm confused.
-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 4/4] drm/i915: Invalidate our pages under memory pressure
On Thu, Apr 17, 2014 at 07:34:31PM +0300, Jani Nikula wrote:
> On Wed, 16 Apr 2014, Robert Beckett wrote:
> > On 25/03/2014 13:23, Chris Wilson wrote:
> >> Try to flush out dirty pages into the swapcache (and from there into the
> >> swapfile) when under memory pressure and forced to drop GEM objects from
> >> memory. In effect, this should just allow us to discard unused pages for
> >> memory reclaim and to start writeback earlier.
> >>
> >> v2: Hugh Dickins warned that explicitly starting writeback from
> >> shrink_slab was prone to deadlocks within shmemfs.
> >>
> >> Cc: Hugh Dickins
> >> Signed-off-by: Chris Wilson
> >> ---
> >> drivers/gpu/drm/i915/i915_gem.c | 38
> >> +++---
> >> 1 file changed, 27 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> >> b/drivers/gpu/drm/i915/i915_gem.c
> >> index 135ee8bd55f6..8287fd6701c6 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -60,7 +60,6 @@ static unsigned long i915_gem_shrinker_scan(struct
> >> shrinker *shrinker,
> >>struct shrink_control *sc);
> >> static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv,
> >> long target);
> >> static unsigned long i915_gem_shrink_all(struct drm_i915_private
> >> *dev_priv);
> >> -static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
> >> static void i915_gem_retire_requests_ring(struct intel_ring_buffer
> >> *ring);
> >>
> >> static bool cpu_cache_is_coherent(struct drm_device *dev,
> >> @@ -1685,12 +1684,16 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev,
> >> void *data,
> >>return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
> >> }
> >>
> >> +static inline int
> >> +i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
> >> +{
> >> + return obj->madv == I915_MADV_DONTNEED;
> >> +}
> >> +
> >> /* Immediately discard the backing storage */
> >> static void
> >> i915_gem_object_truncate(struct drm_i915_gem_object *obj)
> >> {
> >> - struct inode *inode;
> >> -
> >>i915_gem_object_free_mmap_offset(obj);
> >>
> >>if (obj->base.filp == NULL)
> >> @@ -1701,16 +1704,28 @@ i915_gem_object_truncate(struct
> >> drm_i915_gem_object *obj)
> >> * To do this we must instruct the shmfs to drop all of its
> >> * backing pages, *now*.
> >> */
> >> - inode = file_inode(obj->base.filp);
> >> - shmem_truncate_range(inode, 0, (loff_t)-1);
> >> -
> >> + shmem_truncate_range(file_inode(obj->base.filp), 0, (loff_t)-1);
> >>obj->madv = __I915_MADV_PURGED;
> >> }
> >>
> >> -static inline int
> >> -i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
> >> +/* Try to discard unwanted pages */
> >> +static void
> >> +i915_gem_object_invalidate(struct drm_i915_gem_object *obj)
> >> {
> >> - return obj->madv == I915_MADV_DONTNEED;
> >> + struct address_space *mapping;
> >> +
> >> + switch (obj->madv) {
> >> + case I915_MADV_DONTNEED:
> >> + i915_gem_object_truncate(obj);
> >> + case __I915_MADV_PURGED:
> >> + return;
> >> + }
> >> +
> >> + if (obj->base.filp == NULL)
> >> + return;
> >> +
> >> + mapping = file_inode(obj->base.filp)->i_mapping,
> >> + invalidate_mapping_pages(mapping, 0, (loff_t)-1);
> >> }
> >>
> >> static void
> >> @@ -1775,8 +1790,7 @@ i915_gem_object_put_pages(struct drm_i915_gem_object
> >> *obj)
> >>ops->put_pages(obj);
> >>obj->pages = NULL;
> >>
> >> - if (i915_gem_object_is_purgeable(obj))
> >> - i915_gem_object_truncate(obj);
> >> + i915_gem_object_invalidate(obj);
> >>
> >>return 0;
> >> }
> >> @@ -4201,6 +4215,8 @@ void i915_gem_free_object(struct drm_gem_object
> >> *gem_obj)
> >>
> >>if (WARN_ON(obj->pages_pin_count))
> >>obj->pages_pin_count = 0;
> >> + if (obj->madv != __I915_MADV_PURGED)
> >> + obj->madv = I915_MADV_DONTNEED;
> >>i915_gem_object_put_pages(obj);
> >>i915_gem_object_free_mmap_offset(obj);
> >>i915_gem_object_release_stolen(obj);
> >>
> >
> > Functionally it looks good to me.
> >
> > Though, you may want a /* fall-through */ comment (some people cant
> > mentally parse fallthroughs without being prompted) and a default:
> > break; (to avoid any static code analysis complaints) in the switch in
> > i915_gem_object_invalidate.
>
> Or just two if statements.
if (MADV_DONTNEED)
i915_gem_object_truncate(obj);
if (!MADV_WILLNEED)
return;
would indeed looka a bit cleaner.
And a question to Bob: Is your r-b for the entire series or just this
patch? Generally the assumption is that an r-b tag is only for the patch
replied to, except when otherwise stated. Usually people just slap r-b
tags onto patches as they go through a series.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_
Re: [Intel-gfx] [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact
On Thu, Apr 17, 2014 at 04:18:46PM -0700, Volkin, Bradley D wrote: > On Wed, Mar 19, 2014 at 04:13:06AM -0700, Tvrtko Ursulin wrote: > > + bo_ptr = (char *)(((unsigned long)ptr + (PAGE_SIZE - 1)) > > + & ~(PAGE_SIZE - 1)); > > You might add an ALIGN macro in this file or a suitable header. A couple of > .c files in lib/ individually define one already. lib/drmtest.h (with the gtkdoc for it include) is the current grab-bag for such random useful pieces. We already have an ARRAY_SIZE in there. -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: Intel-specific primary plane handling (v4)
On Tue, Apr 22, 2014 at 07:14:22PM +0200, Daniel Vetter wrote: > On Tue, Apr 22, 2014 at 08:18:30AM -0700, Matt Roper wrote: > > On Tue, Apr 22, 2014 at 03:47:37PM +0300, Ville Syrjälä wrote: > > > On Fri, Apr 11, 2014 at 02:13:42PM -0700, Matt Roper wrote: > > ... > > > > + int ret; > > > > + > > > > + /* > > > > +* At the moment we use the same set of setplane restrictions > > > > as the > > > > +* DRM primary plane helper, so go ahead and just call the > > > > helper if > > > > +* the primary plane is already enabled. We only need to take > > > > special > > > > +* action if the primary plane is disabled (something i915 can > > > > do but > > > > +* the generic helper can't). > > > > +*/ > > > > + if (intel_crtc->primary_enabled) > > > > + return drm_primary_helper_update(plane, crtc, fb, > > > > +crtc_x, crtc_y, > > > > +crtc_w, crtc_h, > > > > +src_x, src_y, > > > > +src_w, src_h); > > > > > > Why would we want to call that if we have a custom implementation > > > anyway? > > > > This was something Daniel requested on a previous patch iteration; even > > though we're stuck duplicating most of the checks here for the !enabled > > case, he still wanted to see us call into the helper for the enabled > > case (although this will have to change in the future if/when we want to > > start relaxing some of the tests that the helper does, such as plane > > scaling). > > To clarify my request: I was unhappy with all the duplicated tests we have > and would like some way to share them with the plane helper code. If > there's no sane way to do that, then I'm ok with duplication. > > I'm not sure any more what was the issue with extracting the tests from > the plane helper into a new function and reusing them with i915 though. > -Daniel Ah, okay. I think I may have misunderstood what you were asking for the previous time around. Extracting the tests from the helper into a new function should be doable; I'll include that in my next iteration. Sorry for the confusion. Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Bug 3.15-rc2] [bisected] intel framebuffer broken
On Tue, Apr 22, 2014 at 07:22:40PM +0300, Imre Deak wrote: > On Tue, 2014-04-22 at 18:10 +0300, Jani Nikula wrote: > > Knut, thanks for a quick bisect. > > > > Imre, please have a look at the sdvo WARN related to sysfs links in the > > dmesg. > > It's probably the same WARN fixed by: > http://patchwork.freedesktop.org/patch/24126/ Knut, can you please check that this patch gets rid of the WARN? -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 v3 25/25] drm/i915: vlv: add runtime PM support
Add runtime PM support for VLV, but leave it disabled. The next patch
enables it.
The suspend/resume sequence used is based on [1] and [2]. In practice we
depend on the GT RC6 mechanism to save the HW context depending on the
render and media power wells. By the time we run the runtime suspend
callback the display side is also off and the HW context for that is
managed by the display power domain framework.
Besides the above there are Gunit registers that depend on a system-wide
power well. This power well goes off once the device enters any of the
S0i[R123] states. To handle this scenario, save/restore these Gunit
registers. Note that this is not the complete register set dictated by
[2], to remove some overhead, registers that are known not to be used are
ignored. Also some registers are fully setup by initialization functions
called during resume, these are not saved either. The list of registers
can be further reduced, see the TODO note in the code.
[1] VLV_gfx_clocking_PM_reset_y12w21d3 / "Driver D3 entry/exit"
[2] VLV2_S0IXRegs
v2:
- unchanged
v3:
- fix s/GEN6_PMIIR/GEN6_PMIMR/ typo when saving/restoring registers
(Ville)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.c | 327
drivers/gpu/drm/i915/i915_drv.h | 62
2 files changed, 389 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 31988f6..0a96a5d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -911,6 +911,198 @@ static int hsw_runtime_resume(struct drm_i915_private
*dev_priv)
return 0;
}
+/*
+ * Save all Gunit registers that may be lost after a D3 and a subsequent
+ * S0i[R123] transition. The list of registers needing a save/restore is
+ * defined in the VLV2_S0IXRegs document. This documents marks all Gunit
+ * registers in the following way:
+ * - Driver: saved/restored by the driver
+ * - Punit : saved/restored by the Punit firmware
+ * - No, w/o marking: no need to save/restore, since the register is R/O or
+ *used internally by the HW in a way that doesn't depend
+ *keeping the content across a suspend/resume.
+ * - Debug : used for debugging
+ *
+ * We save/restore all registers marked with 'Driver', with the following
+ * exceptions:
+ * - Registers out of use, including also registers marked with 'Debug'.
+ * These have no effect on the driver's operation, so we don't save/restore
+ * them to reduce the overhead.
+ * - Registers that are fully setup by an initialization function called from
+ * the resume path. For example many clock gating and RPS/RC6 registers.
+ * - Registers that provide the right functionality with their reset defaults.
+ *
+ * TODO: Except for registers that based on the above 3 criteria can be safely
+ * ignored, we save/restore all others, practically treating the HW context as
+ * a black-box for the driver. Further investigation is needed to reduce the
+ * saved/restored registers even further, by following the same 3 criteria.
+ */
+static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
+{
+ struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
+ int i;
+
+ /* GAM 0x4000-0x4770 */
+ s->wr_watermark = I915_READ(GEN7_WR_WATERMARK);
+ s->gfx_prio_ctrl= I915_READ(GEN7_GFX_PRIO_CTRL);
+ s->arb_mode = I915_READ(ARB_MODE);
+ s->gfx_pend_tlb0= I915_READ(GEN7_GFX_PEND_TLB0);
+ s->gfx_pend_tlb1= I915_READ(GEN7_GFX_PEND_TLB1);
+
+ for (i = 0; i < ARRAY_SIZE(s->lra_limits); i++)
+ s->lra_limits[i] = I915_READ(GEN7_LRA_LIMITS_BASE + i * 4);
+
+ s->media_max_req_count = I915_READ(GEN7_MEDIA_MAX_REQ_COUNT);
+ s->gfx_max_req_count= I915_READ(GEN7_MEDIA_MAX_REQ_COUNT);
+
+ s->render_hwsp = I915_READ(RENDER_HWS_PGA_GEN7);
+ s->ecochk = I915_READ(GAM_ECOCHK);
+ s->bsd_hwsp = I915_READ(BSD_HWS_PGA_GEN7);
+ s->blt_hwsp = I915_READ(BLT_HWS_PGA_GEN7);
+
+ s->tlb_rd_addr = I915_READ(GEN7_TLB_RD_ADDR);
+
+ /* MBC 0x9024-0x91D0, 0x8500 */
+ s->g3dctl = I915_READ(GEN7_G3DCTL);
+ s->gsckgctl = I915_READ(GEN7_GSCKGCTL);
+ s->mbctl= I915_READ(GEN6_MBCTL);
+
+ /* GCP 0x9400-0x9424, 0x8100-0x810C */
+ s->ucgctl1 = I915_READ(GEN6_UCGCTL1);
+ s->ucgctl3 = I915_READ(GEN7_UCGCTL3);
+ s->rcgctl1 = I915_READ(GEN7_RCGCTL1);
+ s->rcgctl2 = I915_READ(GEN7_RCGCTL2);
+ s->rstctl = I915_READ(GEN7_RSTCTL);
+ s->misccpctl= I915_READ(GEN7_MISCCPCTL);
+
+ /* GPM 0xA000-0xAA84, 0x8000-0x80FC */
+ s->gfxpause = I915_READ(GEN7_GFXPAUSE);
+ s->rpdeuhwtc= I915_READ(GEN7_RPDEUHWTC);
+ s->rpd
[Intel-gfx] [PATCH v3 19/25] drm/i915: reinit GT power save during resume
During runtime suspend there can be a last pending rps.work, so make
sure it's canceled. Note that in the runtime suspend callback we can't
get any RPS interrupts since it's called only after the GPU goes idle
and we set the minimum RPS frequency. The next possibility for an RPS
interrupt is only after getting an RPM ref (for example because of a new
GPU command) and calling the RPM resume callback.
v2:
- patch introduced in v2 of the patchset
v3:
- Change the order of canceling the rps.work and disabling interrupts to
avoid the race between interrupt disabling and the the rps.work. Race
spotted by Ville.
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b87109c..edd4ab8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -919,6 +919,12 @@ static int intel_runtime_suspend(struct device *device)
DRM_DEBUG_KMS("Suspending device\n");
+ /*
+* rps.work can't be rearmed here, since we get here only after making
+* sure the GPU is idle and the RPS freq is set to the minimum. See
+* intel_mark_idle().
+*/
+ cancel_work_sync(&dev_priv->rps.work);
intel_runtime_pm_disable_interrupts(dev);
if (IS_GEN6(dev))
@@ -970,6 +976,7 @@ static int intel_runtime_resume(struct device *device)
gen6_update_ring_freq(dev);
intel_runtime_pm_restore_interrupts(dev);
+ intel_reset_gt_powersave(dev);
DRM_DEBUG_KMS("Device resumed\n");
return 0;
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] xf86-video-intel: enable hw-generated binding tables
On Tue, Apr 22, 2014 at 04:23:12PM +0100, Chris Wilson wrote: > On Tue, Apr 22, 2014 at 06:16:34PM +0300, Abdiel Janulgue wrote: > > This patch series enables resource streamer for xf86-video-intel UXA. > > > > Fixes i965 Mesa driver that makes use of resource streamer optimization > > to survive a full piglit run without bricking the machine. Previously, > > I get random hungs when doing a full piglit round when enabling RS. > > After months of head-scratching, I noticed I didn't quite pay attention > > to this small detail in bspec: > > > > "The binding table generator feature has a simple all or nothing > > model. If HW generated binding tables are enabled, the driver must enable > > the pool and use 3D_HW_BINDING_TABLE_POINTER_* commands." > > > > I realized that our xf86-video-intel driver is piping out 3D commands > > as well that used the older manual-generated binding table format > > that caused a conflict when Mesa is using the hw-generated binding table > > format. > > This has to be per-context right? Otherwise how do you intend to > coordinate multiple clients submitting to the kernel using incompatible > command sets? Even in the restricted X/DRI sense, how do you intend to > negotiate which to use? Hm, I've thought that the MI_BB_START should synchronize with the asynchronous RS parsing/processing? Is there no way to disable RS again for the next batch in a different context? If there's no way to solve this with contexts or some manual reset trick using LRIs then we're pretty decently screwed up. Worst case we need to stop the gpu and reset it to keep backwards compat :( -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 3/3] drm/i915: add null render states for gen6, gen7 and gen8
These are generated with intel-gpu-tools/tools/null_state_gen.
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_renderstate_gen6.h | 281 ++-
drivers/gpu/drm/i915/intel_renderstate_gen7.h | 245 -
drivers/gpu/drm/i915/intel_renderstate_gen8.h | 471 -
3 files changed, 994 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h
b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
index 4809f2f..5dea85c 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen6.h
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
@@ -2,10 +2,289 @@
#define __INTEL_RENDERSTATE_GEN6
static const uint32_t gen6_null_state_relocs[] = {
+ 0x0020,
+ 0x0024,
+ 0x002c,
+ 0x01e0,
+ 0x01e4,
};
static const uint32_t gen6_null_state_batch[] = {
- MI_BATCH_BUFFER_END,
+ 0x6904,
+ 0x790d0001,
+ 0x,
+ 0x,
+ 0x7818,
+ 0x0001,
+ 0x61010008,
+ 0x,
+ 0x0001, /* reloc */
+ 0x0001, /* reloc */
+ 0x,
+ 0x0001, /* reloc */
+ 0x,
+ 0x0001,
+ 0x,
+ 0x0001,
+ 0x6102,
+ 0x,
+ 0x78050001,
+ 0x0018,
+ 0x,
+ 0x780d1002,
+ 0x,
+ 0x,
+ 0x0420,
+ 0x78150003,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x7814,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x78160003,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x78110005,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x78120002,
+ 0x,
+ 0x,
+ 0x,
+ 0x78170003,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x79050005,
+ 0xe004,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x7910,
+ 0x,
+ 0x7902,
+ 0x,
+ 0x,
+ 0x,
+ 0x780e0002,
+ 0x0441,
+ 0x0401,
+ 0x0401,
+ 0x78021002,
+ 0x,
+ 0x,
+ 0x0400,
+ 0x78130012,
+ 0x00400810,
+ 0x,
+ 0x2000,
+ 0x0400,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x78140007,
+ 0x0280,
+ 0x0808,
+ 0x,
+ 0x0006,
+ 0x4e080002,
+ 0x00100400,
+ 0x,
+ 0x,
+ 0x78090005,
+ 0x0200,
+ 0x,
+ 0x02f6,
+ 0x1133,
+ 0x02850004,
+ 0x1122,
+ 0x78011002,
+ 0x,
+ 0x,
+ 0x0200,
+ 0x78080003,
+ 0x2000,
+ 0x0448, /* reloc */
+ 0x0448, /* reloc */
+ 0x,
+ 0x0500, /* cmds end */
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x0220, /* state start */
+ 0x0240,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x,
+ 0x0060005a,
+ 0x204077be,
+ 0x00c0,
+ 0x008d0040,
+ 0x0060005a,
+ 0x206077be,
+ 0x00c0,
+ 0x008d0080,
+ 0x0060005a,
+ 0x208077be,
+ 0x00d0,
+ 0x008d0040,
+ 0x0060005a,
+ 0x20a077be,
+ 0x00d0,
+ 0x008d0080,
+ 0x0201,
+ 0x20080061,
+ 0x,
+ 0x,
+ 0x0061,
+ 0x20200022,
+ 0x008d,
+ 0x,
+ 0x02800031,
+ 0x21c01cc9,
+ 0x0020,
+ 0x0a8a0001,
+ 0x0061,
+ 0x204003be,
+ 0x008d01c0,
+ 0x,
+ 0x0061,
+ 0x206003be,
+ 0x008d01e0,
+ 0x,
+ 0x0061,
+ 0x208003be,
+
[Intel-gfx] [RFC 0/3] render state initialization (bdw rc6)
Hi, Here are patches to initialize first render context to a hopefully, valid state. If pipeline/context is not initialized and we enter rc6 on bdw, the render ring can hung on the first batch submitted. That is atleast the hypothesis this work is based on. The states are stripped from rendercopy_genX's from i-g-t/lib and the state generators are part of i-g-t also. The states are propably overshoot from what can be consider the minimal valid (null) state on the pipeline. I just initialized everything rendercopy does and haven't really put effort on optimizing the state until I get some test results that this indeed solves anything. The state generators can be found here but they are not needed for testing. http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally untested. Here is the branch for testing: http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state I am interested to know if these patches make matters better/worse for those who have issues with first batch hanging on bdw, but as always, any feedback is greatly appreciated. Mika Kuoppala (3): drm/i915: export vmap_batch from command parser drm/i915: add render state initialization drm/i915: add null render states for gen6, gen7 and gen8 drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_cmd_parser.c|4 +- drivers/gpu/drm/i915/i915_drv.h |3 + drivers/gpu/drm/i915/i915_gem_context.c |7 + drivers/gpu/drm/i915/i915_gem_render_state.c | 222 drivers/gpu/drm/i915/intel_renderstate_gen6.h | 290 +++ drivers/gpu/drm/i915/intel_renderstate_gen7.h | 254 + drivers/gpu/drm/i915/intel_renderstate_gen8.h | 480 + 8 files changed, 1259 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: add render state initialization
HW guys say that it is not a cool idea to let device
go into rc6 without proper 3d pipeline state.
For each new uninitialized context, generate a
valid null render state to be run on context
creation.
This patch introduces a skeleton with emty states.
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/i915_drv.h |2 +
drivers/gpu/drm/i915/i915_gem_context.c |7 +
drivers/gpu/drm/i915/i915_gem_render_state.c | 222 +
drivers/gpu/drm/i915/intel_renderstate_gen6.h | 11 ++
drivers/gpu/drm/i915/intel_renderstate_gen7.h | 11 ++
drivers/gpu/drm/i915/intel_renderstate_gen8.h | 11 ++
7 files changed, 265 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b1445b7..b5d4029 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
# GEM code
i915-y += i915_cmd_parser.o \
i915_gem_context.o \
+ i915_gem_render_state.o \
i915_gem_debug.o \
i915_gem_dmabuf.o \
i915_gem_evict.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43b022c..f6ae2ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev,
void *data,
int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+/* i915_gem_render_state.c */
+int i915_gem_init_render_state(struct intel_ring_buffer *ring);
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev,
struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 30b355a..d648d4d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
/* obj is kept alive until the next request by its active ref */
i915_gem_object_ggtt_unpin(from->obj);
i915_gem_context_unreference(from);
+ } else {
+ if (ring->id == RCS && to->is_initialized == false) {
+
+ ret = i915_gem_init_render_state(ring);
+ if (ret)
+ DRM_ERROR("init render state: %d\n", ret);
+ }
}
to->is_initialized = true;
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
b/drivers/gpu/drm/i915/i915_gem_render_state.c
new file mode 100644
index 000..409f99b
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -0,0 +1,222 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Mika Kuoppala
+ *
+ */
+
+#include "i915_drv.h"
+
+#include "intel_renderstate_gen6.h"
+#include "intel_renderstate_gen7.h"
+#include "intel_renderstate_gen8.h"
+
+#define BATCH_MAX_SIZE 4096
+
+struct i915_render_state {
+ struct drm_i915_gem_object *obj;
+ unsigned long ggtt_offset;
+ void *batch;
+ u32 size;
+ u32 len;
+};
+
+static struct i915_render_state *
+render_state_alloc(struct drm_device *dev, unsigned int size)
+{
+ struct i915_render_state *so;
+ int ret;
+
+ so = kzalloc(sizeof(*so), GFP_KERNEL);
+ if (!so)
+ return ERR_PTR(-ENOMEM);
+
+ so->size = ALIGN
[Intel-gfx] [PATCH 1/3] drm/i915: export vmap_batch from command parser
as need it for generating batch commands and state
in subsequent commit
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_cmd_parser.c |4 ++--
drivers/gpu/drm/i915/i915_drv.h|1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9bac097..5fab893 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -704,7 +704,7 @@ static bool valid_reg(const u32 *table, int count, u32 addr)
return false;
}
-static u32 *vmap_batch(struct drm_i915_gem_object *obj)
+void *i915_gem_vmap_obj(struct drm_i915_gem_object *obj)
{
int i;
void *addr = NULL;
@@ -882,7 +882,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,
return ret;
}
- batch_base = vmap_batch(batch_obj);
+ batch_base = i915_gem_vmap_obj(batch_obj);
if (!batch_base) {
DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n");
i915_gem_object_unpin_pages(batch_obj);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92c3095..43b022c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2413,6 +2413,7 @@ void i915_get_extra_instdone(struct drm_device *dev,
uint32_t *instdone);
const char *i915_cache_level_str(int type);
/* i915_cmd_parser.c */
+void __must_check *i915_gem_vmap_obj(struct drm_i915_gem_object *obj);
int i915_cmd_parser_get_version(void);
void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring);
bool i915_needs_cmd_parser(struct intel_ring_buffer *ring);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: use lane count and link rate from VBT as minimums for eDP
Most likely the minimums for both should be enough for enabling the
native resolution on the eDP.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73539
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76711
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 80e5598d66ed..c613da9cb1a9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -764,8 +764,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc *intel_crtc = encoder->new_crtc;
struct intel_connector *intel_connector = intel_dp->attached_connector;
int lane_count, clock;
+ int min_lane_count = 1;
int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
/* Conveniently, the link BW constants become indices with a shift...*/
+ int min_clock = 0;
int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
int bpp, mode_rate;
static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
@@ -798,19 +800,26 @@ intel_dp_compute_config(struct intel_encoder *encoder,
/* Walk through all bpp values. Luckily they're all nicely spaced with 2
* bpc in between. */
bpp = pipe_config->pipe_bpp;
- if (is_edp(intel_dp) && dev_priv->vbt.edp_bpp &&
- dev_priv->vbt.edp_bpp < bpp) {
- DRM_DEBUG_KMS("clamping bpp for eDP panel to BIOS-provided
%i\n",
- dev_priv->vbt.edp_bpp);
- bpp = dev_priv->vbt.edp_bpp;
+ if (is_edp(intel_dp)) {
+ if (dev_priv->vbt.edp_bpp && dev_priv->vbt.edp_bpp < bpp) {
+ DRM_DEBUG_KMS("clamping bpp for eDP panel to
BIOS-provided %i\n",
+ dev_priv->vbt.edp_bpp);
+ bpp = dev_priv->vbt.edp_bpp;
+ }
+
+ if (dev_priv->vbt.edp_lanes)
+ min_lane_count = min(dev_priv->vbt.edp_lanes,
+max_lane_count);
+ if (dev_priv->vbt.edp_rate)
+ min_clock = min(dev_priv->vbt.edp_rate >> 3, max_clock);
}
for (; bpp >= 6*3; bpp -= 2*3) {
mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
bpp);
- for (lane_count = 1; lane_count <= max_lane_count; lane_count
<<= 1) {
- for (clock = 0; clock <= max_clock; clock++) {
+ for (lane_count = min_lane_count; lane_count <= max_lane_count;
lane_count <<= 1) {
+ for (clock = min_clock; clock <= max_clock; clock++) {
link_clock =
drm_dp_bw_code_to_link_rate(bws[clock]);
link_avail = intel_dp_max_data_rate(link_clock,
lane_count);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: clean up VBT eDP link param decoding
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 52 ---
1 file changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index fba9efd09e87..87a1fa6485be 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -592,47 +592,71 @@ parse_edp(struct drm_i915_private *dev_priv, struct
bdb_header *bdb)
dev_priv->vbt.edp_pps = *edp_pps;
- dev_priv->vbt.edp_rate = edp_link_params->rate ? DP_LINK_BW_2_7 :
- DP_LINK_BW_1_62;
+ switch (edp_link_params->rate) {
+ case EDP_RATE_1_62:
+ dev_priv->vbt.edp_rate = DP_LINK_BW_1_62;
+ break;
+ case EDP_RATE_2_7:
+ dev_priv->vbt.edp_rate = DP_LINK_BW_2_7;
+ break;
+ default:
+ DRM_DEBUG_KMS("VBT has unknown eDP link rate value %u\n",
+ edp_link_params->rate);
+ break;
+ }
+
switch (edp_link_params->lanes) {
- case 0:
+ case EDP_LANE_1:
dev_priv->vbt.edp_lanes = 1;
break;
- case 1:
+ case EDP_LANE_2:
dev_priv->vbt.edp_lanes = 2;
break;
- case 3:
- default:
+ case EDP_LANE_4:
dev_priv->vbt.edp_lanes = 4;
break;
+ default:
+ DRM_DEBUG_KMS("VBT has unknown eDP lane count value %u\n",
+ edp_link_params->lanes);
+ break;
}
+
switch (edp_link_params->preemphasis) {
- case 0:
+ case EDP_PREEMPHASIS_NONE:
dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
break;
- case 1:
+ case EDP_PREEMPHASIS_3_5dB:
dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
break;
- case 2:
+ case EDP_PREEMPHASIS_6dB:
dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
break;
- case 3:
+ case EDP_PREEMPHASIS_9_5dB:
dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
break;
+ default:
+ DRM_DEBUG_KMS("VBT has unknown eDP pre-emphasis value %u\n",
+ edp_link_params->preemphasis);
+ break;
}
+
switch (edp_link_params->vswing) {
- case 0:
+ case EDP_VSWING_0_4V:
dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_400;
break;
- case 1:
+ case EDP_VSWING_0_6V:
dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_600;
break;
- case 2:
+ case EDP_VSWING_0_8V:
dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_800;
break;
- case 3:
+ case EDP_VSWING_1_2V:
dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_1200;
break;
+ default:
+ DRM_DEBUG_KMS("VBT has unknown eDP voltage swing value %u\n",
+ edp_link_params->vswing);
+ break;
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] drm/i915: fix edp link training, hopefully
These are RFC-ish patches to use link params from VBT for eDP, to hopefully fix issues related to preferring fast over wide lane configuration with some eDP panels. Untested, as I don't have hw that actually fails. BR, Jani. Jani Nikula (2): drm/i915: clean up VBT eDP link param decoding drm/i915: use lane count and link rate from VBT as minimums for eDP drivers/gpu/drm/i915/intel_bios.c | 52 --- drivers/gpu/drm/i915/intel_dp.c | 23 +++-- 2 files changed, 54 insertions(+), 21 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v4)
On Tue, Apr 22, 2014 at 08:18:30AM -0700, Matt Roper wrote: > On Tue, Apr 22, 2014 at 03:47:37PM +0300, Ville Syrjälä wrote: > > On Fri, Apr 11, 2014 at 02:13:42PM -0700, Matt Roper wrote: > ... > > > + int ret; > > > + > > > + /* > > > + * At the moment we use the same set of setplane restrictions as the > > > + * DRM primary plane helper, so go ahead and just call the helper if > > > + * the primary plane is already enabled. We only need to take special > > > + * action if the primary plane is disabled (something i915 can do but > > > + * the generic helper can't). > > > + */ > > > + if (intel_crtc->primary_enabled) > > > + return drm_primary_helper_update(plane, crtc, fb, > > > + crtc_x, crtc_y, > > > + crtc_w, crtc_h, > > > + src_x, src_y, > > > + src_w, src_h); > > > > Why would we want to call that if we have a custom implementation > > anyway? > > This was something Daniel requested on a previous patch iteration; even > though we're stuck duplicating most of the checks here for the !enabled > case, he still wanted to see us call into the helper for the enabled > case (although this will have to change in the future if/when we want to > start relaxing some of the tests that the helper does, such as plane > scaling). To clarify my request: I was unhappy with all the duplicated tests we have and would like some way to share them with the plane helper code. If there's no sane way to do that, then I'm ok with duplication. I'm not sure any more what was the issue with extracting the tests from the plane helper into a new function and reusing them with i915 though. -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] tests/gen7_forcewake_mt: Don't set the GGTT bit in SRM command
On Wed, Apr 09, 2014 at 09:47:57AM -0700, Daniel Vetter wrote: > On Wed, Apr 09, 2014 at 08:12:28AM -0700, Volkin, Bradley D wrote: > > On Tue, Apr 08, 2014 at 11:20:30PM -0700, Chris Wilson wrote: > > > On Tue, Apr 08, 2014 at 02:22:16PM -0700, bradley.d.vol...@intel.com > > > wrote: > > > > From: Brad Volkin > > > > > > > > The command parser in newer kernels will reject it and setting this > > > > bit is not required for the actual test case. > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76670 > > > > Signed-off-by: Brad Volkin > > > > > > This was written how I did in the ddx... > > > > Oh, I thought you had said that the ddx didn't use MI_STORE_REGISTER_MEM, > > and > > I didn't see it used in the current ddx code, so I thought that part of the > > test wasn't relevant to the actual workaround. It looked like it was just a > > write so we could see the values and check that they were updated. But if it > > is, then yeah, I don't want to change the test behavior. > > Hm right I think Chris said that in the ended he never released a ddx > version with this code. Now I'm indeed rather confused what's going on. If > we don't need it I obviously prefer less complexity in the kernel cmd > parser. Chris, can you help clarify this? Thanks, Brad > -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 3/4] drm: Introduce drm_fb_helper_prepare()
On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> To implement hotplug detection in a race-free manner, drivers must call
> drm_kms_helper_poll_init() before hotplug events can be triggered. Such
> events can be triggered right after any of the encoders or connectors
> are initialized. At the same time, if the drm_fb_helper_hotplug_event()
> helper is used by a driver, then the poll helper requires some parts of
> the FB helper to be initialized to prevent a crash.
>
> At the same time, drm_fb_helper_init() requires information that is not
> necessarily available at such an early stage (number of CRTCs and
> connectors), so it cannot be used yet.
>
> Add a new helper, drm_fb_helper_prepare(), that initializes the bare
> minimum needed to allow drm_kms_helper_poll_init() to execute and any
> subsequent hotplug events to be processed properly.
>
> Signed-off-by: Thierry Reding
Some bikeshed on the kerneldoc below, with that addressed this is
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/armada/armada_fbdev.c | 2 +-
> drivers/gpu/drm/ast/ast_fb.c | 4 ++-
> drivers/gpu/drm/bochs/bochs_fbdev.c | 3 ++-
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 4 ++-
> drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
> drivers/gpu/drm/drm_fb_helper.c | 43
> ---
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 ++-
> drivers/gpu/drm/gma500/framebuffer.c | 3 ++-
> drivers/gpu/drm/i915/intel_fbdev.c| 3 ++-
> drivers/gpu/drm/mgag200/mgag200_fb.c | 3 ++-
> drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 ++-
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
> drivers/gpu/drm/qxl/qxl_fb.c | 5 +++-
> drivers/gpu/drm/radeon/radeon_fb.c| 4 ++-
> drivers/gpu/drm/tegra/fb.c| 4 +--
> drivers/gpu/drm/udl/udl_fb.c | 3 ++-
> include/drm/drm_fb_helper.h | 2 ++
> 18 files changed, 68 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c
> b/drivers/gpu/drm/armada/armada_fbdev.c
> index 21aa1261dba2..9437e11d5df1 100644
> --- a/drivers/gpu/drm/armada/armada_fbdev.c
> +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> @@ -149,7 +149,7 @@ int armada_fbdev_init(struct drm_device *dev)
>
> priv->fbdev = fbh;
>
> - fbh->funcs = &armada_fb_helper_funcs;
> + drm_fb_helper_prepare(dev, fbh, &armada_fb_helper_funcs);
>
> ret = drm_fb_helper_init(dev, fbh, 1, 1);
> if (ret) {
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 2113894e4ff8..cba45c774552 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -328,8 +328,10 @@ int ast_fbdev_init(struct drm_device *dev)
> return -ENOMEM;
>
> ast->fbdev = afbdev;
> - afbdev->helper.funcs = &ast_fb_helper_funcs;
> spin_lock_init(&afbdev->dirty_lock);
> +
> + drm_fb_helper_prepare(dev, &afbdev->helper, &ast_fb_helper_funcs);
> +
> ret = drm_fb_helper_init(dev, &afbdev->helper,
>1, 1);
> if (ret) {
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 17e5c17f2730..19cf3e9413b6 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -189,7 +189,8 @@ int bochs_fbdev_init(struct bochs_device *bochs)
> {
> int ret;
>
> - bochs->fb.helper.funcs = &bochs_fb_helper_funcs;
> + drm_fb_helper_prepare(bochs->dev, &bochs->fb.helper,
> + &bochs_fb_helper_funcs);
>
> ret = drm_fb_helper_init(bochs->dev, &bochs->fb.helper,
>1, 1);
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 2bd0291168e4..2a135f253e29 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -306,9 +306,11 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
> return -ENOMEM;
>
> cdev->mode_info.gfbdev = gfbdev;
> - gfbdev->helper.funcs = &cirrus_fb_helper_funcs;
> spin_lock_init(&gfbdev->dirty_lock);
>
> + drm_fb_helper_prepare(cdev->dev, &gfbdev->helper,
> + &cirrus_fb_helper_funcs);
> +
> ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
>cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
> if (ret) {
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index b74f9e58b69d..acbbd230e081 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -354,9 +354,10 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct
> drm_device *dev,
> return ERR_PTR(-ENOMEM);
> }
>
> - fbdev_cma->fb_helper.funcs = &drm_fb_c
Re: [Intel-gfx] [PATCH 2/4] drm: Constify struct drm_fb_helper_funcs
On Tue, Apr 22, 2014 at 04:42:19PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> There's no need for this to be modifiable. Make it const so that it can
> be put into the .rodata section.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/armada/armada_fbdev.c | 2 +-
> drivers/gpu/drm/ast/ast_fb.c | 2 +-
> drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 +-
> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
> drivers/gpu/drm/gma500/framebuffer.c | 2 +-
> drivers/gpu/drm/i915/intel_fbdev.c| 2 +-
> drivers/gpu/drm/mgag200/mgag200_fb.c | 2 +-
> drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
> drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
> drivers/gpu/drm/radeon/radeon_fb.c| 2 +-
> drivers/gpu/drm/tegra/fb.c| 2 +-
> drivers/gpu/drm/udl/udl_fb.c | 2 +-
> include/drm/drm_fb_helper.h | 2 +-
> 17 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c
> b/drivers/gpu/drm/armada/armada_fbdev.c
> index 948cb14c561e..21aa1261dba2 100644
> --- a/drivers/gpu/drm/armada/armada_fbdev.c
> +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> @@ -131,7 +131,7 @@ static int armada_fb_probe(struct drm_fb_helper *fbh,
> return ret;
> }
>
> -static struct drm_fb_helper_funcs armada_fb_helper_funcs = {
> +static const struct drm_fb_helper_funcs armada_fb_helper_funcs = {
> .gamma_set = armada_drm_crtc_gamma_set,
> .gamma_get = armada_drm_crtc_gamma_get,
> .fb_probe = armada_fb_probe,
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index a28640f47c27..2113894e4ff8 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -287,7 +287,7 @@ static void ast_fb_gamma_get(struct drm_crtc *crtc, u16
> *red, u16 *green,
> *blue = ast_crtc->lut_b[regno] << 8;
> }
>
> -static struct drm_fb_helper_funcs ast_fb_helper_funcs = {
> +static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
> .gamma_set = ast_fb_gamma_set,
> .gamma_get = ast_fb_gamma_get,
> .fb_probe = astfb_create,
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 561b84474122..17e5c17f2730 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -179,7 +179,7 @@ void bochs_fb_gamma_get(struct drm_crtc *crtc, u16 *red,
> u16 *green,
> *blue = regno;
> }
>
> -static struct drm_fb_helper_funcs bochs_fb_helper_funcs = {
> +static const struct drm_fb_helper_funcs bochs_fb_helper_funcs = {
> .gamma_set = bochs_fb_gamma_set,
> .gamma_get = bochs_fb_gamma_get,
> .fb_probe = bochsfb_create,
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 32bbba0a787b..2bd0291168e4 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -288,7 +288,7 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
> return 0;
> }
>
> -static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
> +static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
> .gamma_set = cirrus_crtc_fb_gamma_set,
> .gamma_get = cirrus_crtc_fb_gamma_get,
> .fb_probe = cirrusfb_create,
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 61b5a47ad239..b74f9e58b69d 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -327,7 +327,7 @@ err_drm_gem_cma_free_object:
> return ret;
> }
>
> -static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
> +static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
> .fb_probe = drm_fbdev_cma_create,
> };
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index addbf7536da4..7ccf04917f47 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -233,7 +233,7 @@ out:
> return ret;
> }
>
> -static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
> +static const struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
> .fb_probe = exynos_drm_fbdev_create,
> };
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> b/drivers/gpu/drm/gma500/framebuffer.c
> index e7fcc148f333..76e4d777d01d 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -561,7 +561,7 @@ static int psbfb_probe(struct drm_fb_helper *helper,
> return psbfb_create(psb_fbdev, sizes);
> }
>
> -static struct drm_fb_helper_fu
Re: [Intel-gfx] [RFC] xf86-video-intel: enable hw-generated binding tables
On Tue, Apr 22, 2014 at 06:16:34PM +0300, Abdiel Janulgue wrote: > This patch series enables resource streamer for xf86-video-intel UXA. > > Fixes i965 Mesa driver that makes use of resource streamer optimization > to survive a full piglit run without bricking the machine. Previously, > I get random hungs when doing a full piglit round when enabling RS. > After months of head-scratching, I noticed I didn't quite pay attention > to this small detail in bspec: > > "The binding table generator feature has a simple all or nothing > model. If HW generated binding tables are enabled, the driver must enable > the pool and use 3D_HW_BINDING_TABLE_POINTER_* commands." > > I realized that our xf86-video-intel driver is piping out 3D commands > as well that used the older manual-generated binding table format > that caused a conflict when Mesa is using the hw-generated binding table > format. This has to be per-context right? Otherwise how do you intend to coordinate multiple clients submitting to the kernel using incompatible command sets? Even in the restricted X/DRI sense, how do you intend to negotiate which to use? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v4)
On Tue, Apr 22, 2014 at 03:47:37PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 11, 2014 at 02:13:42PM -0700, Matt Roper wrote:
...
> > + int ret;
> > +
> > + /*
> > +* At the moment we use the same set of setplane restrictions as the
> > +* DRM primary plane helper, so go ahead and just call the helper if
> > +* the primary plane is already enabled. We only need to take special
> > +* action if the primary plane is disabled (something i915 can do but
> > +* the generic helper can't).
> > +*/
> > + if (intel_crtc->primary_enabled)
> > + return drm_primary_helper_update(plane, crtc, fb,
> > +crtc_x, crtc_y,
> > +crtc_w, crtc_h,
> > +src_x, src_y,
> > +src_w, src_h);
>
> Why would we want to call that if we have a custom implementation
> anyway?
This was something Daniel requested on a previous patch iteration; even
though we're stuck duplicating most of the checks here for the !enabled
case, he still wanted to see us call into the helper for the enabled
case (although this will have to change in the future if/when we want to
start relaxing some of the tests that the helper does, such as plane
scaling).
...
> > + /*
> > +* pipe_set_base() adjusts crtc->primary->fb; however the DRM setplane
> > +* code that called us expects to handle the framebuffer update and
> > +* reference counting; save and restore the current fb before
> > +* calling it.
> > +*/
> > + tmpfb = plane->fb;
> > + ret = intel_pipe_set_base(crtc, src_x, src_y, fb);
> > + if (ret)
> > + return ret;
> > + plane->fb = tmpfb;
> > +
> > + intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
> > + intel_crtc->pipe);
>
> Needs a primary_enabled check (+ a visibility check for the fully
> clipped case).
If primary_enabled, we already called directly into the helper and
returned up at the top of the function.
I'll go ahead and add the visibility test farther up the function
though.
...
> > +static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> > + int pipe)
> > +{
> > + struct intel_plane *primary;
> > + const uint32_t *intel_primary_formats;
> > + int num_formats;
> > +
> > + primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> > + if (primary == NULL)
> > + return NULL;
> > +
> > + primary->can_scale = false;
> > + primary->pipe = pipe;
> > + primary->plane = pipe;
>
> We need to handle the pre-gen4 fbc primary plane swapping somewhere. I
> guess we still have intel_crtc->plane as well. That should probably get
> converted to use crtc->primary->plane instead.
Hmm, this is something I'm not terribly familiar with. I'm guessing I
just need to copy the logic from intel_crtc_init()?
I can write a Coccinelle patch to replace the intel_crtc->plane with
crtc->primary->plane as well; thanks for pointing that out.
Thanks for the review; I'll send along a new patch that incorporates
your other feedback shortly (and I think you've pointed out a few things
here that apply to the primary plane helper code too). I'm still
reworking my i-g-t patches for this functionality as well, but I've been
traveling lately and haven't had too much time to work on it.
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] xf86-video-intel: enable hw-generated binding tables
This patch series enables resource streamer for xf86-video-intel UXA. Fixes i965 Mesa driver that makes use of resource streamer optimization to survive a full piglit run without bricking the machine. Previously, I get random hungs when doing a full piglit round when enabling RS. After months of head-scratching, I noticed I didn't quite pay attention to this small detail in bspec: "The binding table generator feature has a simple all or nothing model. If HW generated binding tables are enabled, the driver must enable the pool and use 3D_HW_BINDING_TABLE_POINTER_* commands." I realized that our xf86-video-intel driver is piping out 3D commands as well that used the older manual-generated binding table format that caused a conflict when Mesa is using the hw-generated binding table format. I didn't touch the SNA and video acceleration paths yet. Please let me know if I'm on the right track. To use the feature, set in xorg.conf: Option "ResourceStreamer" "true" src/intel_options.c |1 + src/intel_options.h |1 + src/uxa/i965_3d.c |5 ++- src/uxa/i965_reg.h |8 + src/uxa/i965_render.c | 78 +++ src/uxa/intel.h |3 ++ src/uxa/intel_batchbuffer.c |7 ++-- src/uxa/intel_driver.c | 10 +- 8 files changed, 94 insertions(+), 19 deletions(-) Abdiel Janulgue (2): [PATCH 1/2] xf86-video-intel: Add "ResourceStreamer" option [PATCH 2/2] xf86-video-intel: Enable hw-generated binding tables for ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 1/2] xf86-video-intel: Add "ResourceStreamer" option
Signed-off-by: Abdiel Janulgue
---
src/intel_options.c|1 +
src/intel_options.h|1 +
src/uxa/intel.h|3 +++
src/uxa/intel_driver.c | 10 +-
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/intel_options.c b/src/intel_options.c
index 02a4ae1..cc418d1 100644
--- a/src/intel_options.c
+++ b/src/intel_options.c
@@ -35,6 +35,7 @@ const OptionInfoRec intel_options[] = {
{OPTION_DEBUG_FLUSH_CACHES, "DebugFlushCaches", OPTV_BOOLEAN, {0}, 0},
{OPTION_DEBUG_WAIT, "DebugWait", OPTV_BOOLEAN, {0}, 0},
{OPTION_BUFFER_CACHE, "BufferCache", OPTV_BOOLEAN, {0},1},
+ {OPTION_RESOURCESTREAMER, "ResourceStreamer", OPTV_BOOLEAN, {0},
0},
#endif
{-1,NULL, OPTV_NONE, {0},0}
};
diff --git a/src/intel_options.h b/src/intel_options.h
index 77f0c45..4867419 100644
--- a/src/intel_options.h
+++ b/src/intel_options.h
@@ -43,6 +43,7 @@ enum intel_options {
OPTION_DEBUG_FLUSH_CACHES,
OPTION_DEBUG_WAIT,
OPTION_BUFFER_CACHE,
+ OPTION_RESOURCESTREAMER,
#endif
NUM_OPTIONS,
};
diff --git a/src/uxa/intel.h b/src/uxa/intel.h
index 6ac770e..611eeea 100644
--- a/src/uxa/intel.h
+++ b/src/uxa/intel.h
@@ -349,6 +349,9 @@ typedef struct intel_screen_private {
*/
Bool fallback_debug;
unsigned debug_flush;
+
+ dri_bo *hw_bt_pool_bo;
+ Bool use_resource_streamer;
#if HAVE_UDEV
struct udev_monitor *uevent_monitor;
pointer uevent_handler;
diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
index 0a4fe2a..5574ffe 100644
--- a/src/uxa/intel_driver.c
+++ b/src/uxa/intel_driver.c
@@ -196,6 +196,15 @@ static Bool I830GetEarlyOptions(ScrnInfoPtr scrn)
intel->fallback_debug = xf86ReturnOptValBool(intel->Options,
OPTION_FALLBACKDEBUG,
FALSE);
+ intel->use_resource_streamer = xf86ReturnOptValBool(intel->Options,
+OPTION_RESOURCESTREAMER,
+FALSE);
+ if (intel->use_resource_streamer)
+ xf86DrvMsg(scrn->scrnIndex, X_CONFIG,
+ "Enabling Resource Streamer\n");
+ else
+ xf86DrvMsg(scrn->scrnIndex, X_CONFIG,
+ "Disabling Resource Streamer\n");
intel->debug_flush = 0;
@@ -870,7 +879,6 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL)
* appropriate.
*/
intel->XvEnabled = TRUE;
-
#ifdef DRI2
if (intel->directRenderingType == DRI_NONE
&& I830DRI2ScreenInit(screen))
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 2/2] xf86-video-intel: Enable hw-generated binding tables for UXA
Code is based on my hw-generated binding table code for Mesa
adapted to i965_composite path in UXA.
Signed-off-by: Abdiel Janulgue
---
src/uxa/i965_3d.c |5 ++-
src/uxa/i965_reg.h |8 +
src/uxa/i965_render.c | 78 +++
src/uxa/intel_batchbuffer.c |7 ++--
4 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/src/uxa/i965_3d.c b/src/uxa/i965_3d.c
index 757a979..afbb5a7 100644
--- a/src/uxa/i965_3d.c
+++ b/src/uxa/i965_3d.c
@@ -406,7 +406,10 @@ gen7_upload_binding_table(intel_screen_private *intel,
uint32_t ps_binding_table_offset)
{
OUT_BATCH(GEN7_3DSTATE_BINDING_TABLE_POINTERS_PS | (2 - 2));
- OUT_BATCH(ps_binding_table_offset);
+ if (intel->use_resource_streamer)
+ OUT_BATCH(ps_binding_table_offset >> 1);
+ else
+ OUT_BATCH(ps_binding_table_offset);
}
void
diff --git a/src/uxa/i965_reg.h b/src/uxa/i965_reg.h
index a934a67..157b212 100644
--- a/src/uxa/i965_reg.h
+++ b/src/uxa/i965_reg.h
@@ -296,6 +296,14 @@
/* DW1 */
# define GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT 16
+/* GEN7+ resource streamer */
+#define GEN7_3DSTATE_BINDING_TABLE_POOL_ALLOC BRW_3D(3, 1, 0x19)
+# define BINDING_TABLE_POOL_ENABLE 0x0860
+#define GEN7_3DSTATE_BINDING_TABLE_EDIT_VS BRW_3D(3, 0, 0x43)
+#define GEN7_3DSTATE_BINDING_TABLE_EDIT_GS BRW_3D(3, 0, 0x44)
+#define GEN7_3DSTATE_BINDING_TABLE_EDIT_HS BRW_3D(3, 0, 0x45)
+#define GEN7_3DSTATE_BINDING_TABLE_EDIT_DS BRW_3D(3, 0, 0x46)
+#define GEN7_3DSTATE_BINDING_TABLE_EDIT_PS BRW_3D(3, 0, 0x47)
#define PIPELINE_SELECT_3D 0
#define PIPELINE_SELECT_MEDIA 1
diff --git a/src/uxa/i965_render.c b/src/uxa/i965_render.c
index 74f57af..d5225dd 100644
--- a/src/uxa/i965_render.c
+++ b/src/uxa/i965_render.c
@@ -1783,6 +1783,10 @@ static void i965_surface_flush(struct
intel_screen_private *intel)
sizeof(intel->surface_data), 4096);
assert(intel->surface_bo);
+ drm_intel_bo_unreference(intel->hw_bt_pool_bo);
+ intel->hw_bt_pool_bo = drm_intel_bo_alloc(intel->bufmgr, "hw_bt",
+ 131072, 4096);
+ assert(intel->hw_bt_pool_bo);
return;
(void)ret;
}
@@ -2217,32 +2221,70 @@ static void i965_select_vertex_buffer(struct
intel_screen_private *intel)
static void i965_bind_surfaces(struct intel_screen_private *intel)
{
uint32_t *binding_table;
+ uint32_t surf0 = 0, surf1 = 0, surf2 = 0;
assert(intel->surface_used + 4 * SURFACE_STATE_PADDED_SIZE <=
sizeof(intel->surface_data));
- binding_table = (uint32_t*) (intel->surface_data + intel->surface_used);
- intel->surface_table = intel->surface_used;
- intel->surface_used += SURFACE_STATE_PADDED_SIZE;
-
- binding_table[0] =
- i965_set_picture_surface_state(intel,
+ surf0 = i965_set_picture_surface_state(intel,
intel->render_dest_picture,
intel->render_dest,
TRUE);
- binding_table[1] =
- i965_set_picture_surface_state(intel,
+ surf1 = i965_set_picture_surface_state(intel,
intel->render_source_picture,
intel->render_source,
FALSE);
if (intel->render_mask) {
- binding_table[2] =
- i965_set_picture_surface_state(intel,
-
intel->render_mask_picture,
- intel->render_mask,
- FALSE);
+ surf2 = i965_set_picture_surface_state(intel,
+
intel->render_mask_picture,
+ intel->render_mask,
+ FALSE);
+ }
+
+ if (intel->use_resource_streamer) {
+ intel->surface_table += (256 * sizeof(uint16_t));
+ OUT_BATCH(GEN7_3DSTATE_BINDING_TABLE_EDIT_PS | (5 - 2));
+ OUT_BATCH(0x3);
+ {
+ OUT_BATCH(0 << 16 | surf0 >> 5);
+ OUT_BATCH(1 << 16 | surf1 >> 5);
+ OUT_BATCH(2 << 16 | surf2 >> 5);
+ }
+ } else {
+ binding_table = (uint32_t*) (intel->surface_data +
intel->surface_used);
+ intel->surface_table = intel->surface_used;
+ intel->surface_used += SURFACE_STATE_PADDED_SIZE;
+
+ binding_table[0] = surf0;
+ b
Re: [Intel-gfx] Design review request: DRM color manager
David, My apologies for starting a pre-mature design discussion. Daniel, Thanks for pointing out first two things, It was not known to me, I will take care of this in future. First time I presented color-manager design, in internal display design forum, where most of the reviewers were not there. We took the feedback from people who were present, and implemented the design. When we shared color manager implementation, that design was rejected and one of the feedbacks was that it would be better to discuss it on dri-devel where people outside Intel can give their opinion, and that’s the only reason why I added dri-devel for the new design (Please see the attached mail, I replied to all who were in last communication). Please let me know how do we want to proceed now. Regards Shashank -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, April 22, 2014 7:18 PM To: Sharma, Shashank Cc: David Herrmann; intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Thierry Reding; Cn, Ramakrishnan; Alex Deucher; Jindal, Sonika; Shankar, Uma Subject: Re: [Intel-gfx] Design review request: DRM color manager On Tue, Apr 22, 2014 at 12:07:41PM +, Sharma, Shashank wrote: > Thanks again David, > Comments inline. Three things: - Please don't send out .pptx files to upstream/public mailing lists, that's just not how the upstream community works. - Please either fix up ms outlook to do proper in-line quoting or switch to a proper mail client for discussions on dri-devel. I'm ok with this on intel-gfx to some extend since that's our own turf, but on dri-devel the usual rules apply. - I think we should discuss this internally first or at least just on intel-gfx. David, thanks for taking a look at this but imo this shouldn't have escaped yet to the public. My apologies for wasting your time trying to review this proposal. Thanks, Daniel > > Regards > Shashank > -Original Message- > From: David Herrmann [mailto:dh.herrm...@gmail.com] > Sent: Tuesday, April 22, 2014 5:10 PM > To: Sharma, Shashank > Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; > Ville Syrjälä; Thierry Reding; Alex Deucher; Sean Paul; > robdcl...@gmail.com; Mukherjee, Indranil; Jindal, Sonika; Korjani, > Vikas; Shankar, Uma; Cn, Ramakrishnan > Subject: Re: Design review request: DRM color manager > > Hi > > On Tue, Apr 22, 2014 at 12:21 PM, Sharma, Shashank > wrote: > > 1) Why do you register only a single property? Why not register a separate > > property for each color-correction that is available? This way you can drop > > the property-id and use the high-level DRM-prop IDs/names. > >>> That’s the whole idea of color manager. If we keep on creating properties > >>> for each color correction, there would be a big list and a lot of > >>> properties will be exposed. Instead one common blob which can represent > >>> all the properties, correction values and identifiers. It would be easy > >>> to club with atomic modeset kind-of designs also I believe. > > Where is the difference? With one _well-defined_ property for each type we > simply move the identification one level up. With your approach you just move > the type-id one level down into the blob. > > Or in other words: Where is the difference between calling > SetProperty() n-times, or calling it once but with a parameter describing > n-properties? With atomic-modesetting we can set as many properties as we > want and make the kernel apply them atomically. > > >>> Actually we also do not want to populate the property space also, as if > >>> there are 10 color correction methods possible for a hardware, we might > >>> end up listing 10 properties. And there won't be common properties > >>> across all the hardwares also. For example, Hardware A can have > >>> properties X Y Z but Hardware B can have W X and Z. This will make the > >>> property space inconsistent. But if we provide one common interface which > >>> will cover for all the properties, for all the hardwares in a single > >>> blob. The driver will dynamically register its property, in its own > >>> preferred name. A get_prop() will always list down all the supported > >>> color property by this hardware and driver. > > > 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC > > and/or plane. Isn't that enough information for the driver? > >>> This is to make it HW agonist. Actually that's CRTC ID / Plane ID / PIPE > >>> ID / all together an identifier. For example if I want to set gamma > >>> correction for sprite planes only, not on primary plane or pipe level, on > >>> VLV, its possible. This gives me flexibility to mention fine-tuned > >>> correction even in a CRTC. The driver's .enable method can take decision > >>> on this identifier based on the hardware capabilities. > > Yeah, but I meant the drmModeObjectSetProperty() ioctl already tale
Re: [Intel-gfx] [PATCH 17/48] drm/i915: Track which ring a context ran on
On Tue, Apr 22, 2014 at 04:25:22PM +0200, Daniel Vetter wrote: > I think this doesn't have the possibility to have broken anything yet > since we don't allow the same context on multiple rings. Except the > default one, but mesa creates new contexts anyway afaik. Or am I wrong? It's a minor regression in that we reload the context before every batch; a brief wtf when doing perf monitoring. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm: Constify struct drm_fb_helper_funcs
From: Thierry Reding
There's no need for this to be modifiable. Make it const so that it can
be put into the .rodata section.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/armada/armada_fbdev.c | 2 +-
drivers/gpu/drm/ast/ast_fb.c | 2 +-
drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 +-
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
drivers/gpu/drm/gma500/framebuffer.c | 2 +-
drivers/gpu/drm/i915/intel_fbdev.c| 2 +-
drivers/gpu/drm/mgag200/mgag200_fb.c | 2 +-
drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +-
drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
drivers/gpu/drm/radeon/radeon_fb.c| 2 +-
drivers/gpu/drm/tegra/fb.c| 2 +-
drivers/gpu/drm/udl/udl_fb.c | 2 +-
include/drm/drm_fb_helper.h | 2 +-
17 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c
b/drivers/gpu/drm/armada/armada_fbdev.c
index 948cb14c561e..21aa1261dba2 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -131,7 +131,7 @@ static int armada_fb_probe(struct drm_fb_helper *fbh,
return ret;
}
-static struct drm_fb_helper_funcs armada_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs armada_fb_helper_funcs = {
.gamma_set = armada_drm_crtc_gamma_set,
.gamma_get = armada_drm_crtc_gamma_get,
.fb_probe = armada_fb_probe,
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index a28640f47c27..2113894e4ff8 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -287,7 +287,7 @@ static void ast_fb_gamma_get(struct drm_crtc *crtc, u16
*red, u16 *green,
*blue = ast_crtc->lut_b[regno] << 8;
}
-static struct drm_fb_helper_funcs ast_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
.gamma_set = ast_fb_gamma_set,
.gamma_get = ast_fb_gamma_get,
.fb_probe = astfb_create,
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 561b84474122..17e5c17f2730 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -179,7 +179,7 @@ void bochs_fb_gamma_get(struct drm_crtc *crtc, u16 *red,
u16 *green,
*blue = regno;
}
-static struct drm_fb_helper_funcs bochs_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs bochs_fb_helper_funcs = {
.gamma_set = bochs_fb_gamma_set,
.gamma_get = bochs_fb_gamma_get,
.fb_probe = bochsfb_create,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 32bbba0a787b..2bd0291168e4 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -288,7 +288,7 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
return 0;
}
-static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
.gamma_set = cirrus_crtc_fb_gamma_set,
.gamma_get = cirrus_crtc_fb_gamma_get,
.fb_probe = cirrusfb_create,
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c
index 61b5a47ad239..b74f9e58b69d 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -327,7 +327,7 @@ err_drm_gem_cma_free_object:
return ret;
}
-static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
+static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
.fb_probe = drm_fbdev_cma_create,
};
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index addbf7536da4..7ccf04917f47 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -233,7 +233,7 @@ out:
return ret;
}
-static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
.fb_probe = exynos_drm_fbdev_create,
};
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index e7fcc148f333..76e4d777d01d 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -561,7 +561,7 @@ static int psbfb_probe(struct drm_fb_helper *helper,
return psbfb_create(psb_fbdev, sizes);
}
-static struct drm_fb_helper_funcs psb_fb_helper_funcs = {
+static const struct drm_fb_helper_funcs psb_fb_helper_funcs = {
.gamma_set = psbfb_gamma_set,
.gamma_get = psbfb_gamma_get,
.fb_probe = psbfb_probe,
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b
[Intel-gfx] [PATCH 4/4] drm/tegra: Implement race-free hotplug detection
From: Thierry Reding
A race condition currently exists on Tegra, where it can happen that a
monitor attached via HDMI isn't detected during the initial FB helper
setup, but the hotplug event happens too early to be processed by the
poll helpers because they haven't been initialized yet. This happens
because on some boards the HDMI driver can control the regulator that
supplies the +5V pin on the HDMI connector. Therefore depending on the
timing between the initialization of the HDMI driver and the rest of
DRM, it's possible that the monitor returns the hotplug signal right
within the window where we would miss it.
Unfortunately, drm_kms_helper_poll_init() will wreak havoc when called
before at least some parts of the FB helpers have been set up.
This commit fixes this by splitting out the minimum of initialization
required to make drm_kms_helper_poll_init() work into a separate
function that can be called early. It is then safe to move all of the
poll helper initialization to an earlier point in time (before the
HDMI output driver has a chance to enable the +5V supply). That way if
the hotplug signal is returned before the initial FB helper setup, the
monitor will be forcefully detected at that point, and if the hotplug
signal is returned after that it will be properly handled by the poll
helpers.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/drm.c | 8 ++--
drivers/gpu/drm/tegra/drm.h | 1 +
drivers/gpu/drm/tegra/fb.c | 47 ++---
3 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 6f5b6e2f552e..4d36debe3de6 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -41,6 +41,12 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
drm_mode_config_init(drm);
+ err = tegra_drm_fb_prepare(drm);
+ if (err < 0)
+ return err;
+
+ drm_kms_helper_poll_init(drm);
+
err = host1x_device_init(device);
if (err < 0)
return err;
@@ -60,8 +66,6 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
if (err < 0)
return err;
- drm_kms_helper_poll_init(drm);
-
return 0;
}
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 126332c3ecbb..c2d9705de1f9 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -288,6 +288,7 @@ struct tegra_bo *tegra_fb_get_plane(struct drm_framebuffer
*framebuffer,
unsigned int index);
bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer);
bool tegra_fb_is_tiled(struct drm_framebuffer *framebuffer);
+int tegra_drm_fb_prepare(struct drm_device *drm);
extern int tegra_drm_fb_init(struct drm_device *drm);
extern void tegra_drm_fb_exit(struct drm_device *drm);
#ifdef CONFIG_DRM_TEGRA_FBDEV
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 2e3758542c89..70d0e07d353c 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -271,13 +271,9 @@ static const struct drm_fb_helper_funcs
tegra_fb_helper_funcs = {
.fb_probe = tegra_fbdev_probe,
};
-static struct tegra_fbdev *tegra_fbdev_create(struct drm_device *drm,
- unsigned int preferred_bpp,
- unsigned int num_crtc,
- unsigned int max_connectors)
+static struct tegra_fbdev *tegra_fbdev_create(struct drm_device *drm)
{
struct tegra_fbdev *fbdev;
- int err;
fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
if (!fbdev) {
@@ -287,10 +283,21 @@ static struct tegra_fbdev *tegra_fbdev_create(struct
drm_device *drm,
drm_fb_helper_prepare(drm, &fbdev->base, &tegra_fb_helper_funcs);
+ return fbdev;
+}
+
+static int tegra_fbdev_init(struct tegra_fbdev *fbdev,
+ unsigned int preferred_bpp,
+ unsigned int num_crtc,
+ unsigned int max_connectors)
+{
+ struct drm_device *drm = fbdev->base.dev;
+ int err;
+
err = drm_fb_helper_init(drm, &fbdev->base, num_crtc, max_connectors);
if (err < 0) {
dev_err(drm->dev, "failed to initialize DRM FB helper\n");
- goto free;
+ return err;
}
err = drm_fb_helper_single_add_all_connectors(&fbdev->base);
@@ -299,21 +306,17 @@ static struct tegra_fbdev *tegra_fbdev_create(struct
drm_device *drm,
goto fini;
}
- drm_helper_disable_unused_functions(drm);
-
err = drm_fb_helper_initial_config(&fbdev->base, preferred_bpp);
if (err < 0) {
dev_err(drm->dev, "failed to set initial configuration\n");
goto fini;
}
- return fbdev;
+ return 0;
[Intel-gfx] [PATCH 1/4] drm/fb-helper: Fix hpd vs. initial config races
From: Daniel Vetter
Some drivers need to be able to have a perfect race-free fbcon setup.
Current drivers only enable hotplug processing after the call to
drm_fb_helper_initial_config which leaves a tiny but important race.
This race is especially noticable on embedded platforms where the
driver itself enables the voltage for the hdmi output, since only then
will monitors (after a bit of delay, as usual) respond by asserting
the hpd pin.
Most of the infrastructure is already there with the split-out
drm_fb_helper_init. And drm_fb_helper_initial_config already has all
the required locking to handle concurrent hpd events since
commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f
Author: Daniel Vetter
Date: Thu Mar 20 14:26:35 2014 +0100
drm/fb-helper: improve drm_fb_helper_initial_config locking
The only missing bit is making drm_fb_helper_hotplug_event save
against concurrent calls of drm_fb_helper_initial_config. The only
unprotected bit is the check for fb_helper->fb.
With that drivers can first initialize the fb helper, then enabel
hotplug processing and then set up the initial config all in a
completely race-free manner. Update kerneldoc and convert i915 as a
proof of concept.
Feature requested by Thierry since his tegra driver atm reliably boots
slowly enough to misses the hotplug event for an external hdmi screen,
but also reliably boots to quickly for the hpd pin to be asserted when
the fb helper calls into the hdmi ->detect function.
Cc: Thierry Reding
Signed-off-by: Daniel Vetter
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_fb_helper.c | 11 +--
drivers/gpu/drm/i915/i915_dma.c | 3 ---
drivers/gpu/drm/i915/i915_drv.c | 2 --
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/i915_irq.c | 4
5 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e95ed5805f07..80ce92ab91f9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1587,8 +1587,10 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
* either the output polling work or a work item launched from the driver's
* hotplug interrupt).
*
- * Note that the driver must ensure that this is only called _after_ the fb has
- * been fully set up, i.e. after the call to drm_fb_helper_initial_config.
+ * Note that drivers may call this even before calling
+ * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This allows
+ * for a race-free fbcon setup and will make sure that the fbdev emulation will
+ * not miss any hotplug events.
*
* RETURNS:
* 0 on success and a non-zero error code otherwise.
@@ -1598,11 +1600,8 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper
*fb_helper)
struct drm_device *dev = fb_helper->dev;
u32 max_width, max_height;
- if (!fb_helper->fb)
- return 0;
-
mutex_lock(&fb_helper->dev->mode_config.mutex);
- if (!drm_fb_helper_is_bound(fb_helper)) {
+ if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
fb_helper->delayed_hotplug = true;
mutex_unlock(&fb_helper->dev->mode_config.mutex);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index cc0c6eded51c..89ba88d37ae1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1375,9 +1375,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
*/
intel_fbdev_initial_config(dev);
- /* Only enable hotplug handling once the fbdev is fully set up. */
- dev_priv->enable_hotplug_processing = true;
-
drm_kms_helper_poll_init(dev);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 254b3236200b..dee36a5b7fae 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -448,7 +448,6 @@ static int i915_drm_freeze(struct drm_device *dev)
cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
drm_irq_uninstall(dev);
- dev_priv->enable_hotplug_processing = false;
/*
* Disable CRTCs directly since we want to preserve sw state
* for _thaw.
@@ -589,7 +588,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
* notifications.
* */
intel_hpd_init(dev);
- dev_priv->enable_hotplug_processing = true;
/* Config may have changed between suspend and resume */
intel_resume_hotplug(dev);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7d6acb401fd9..41094d6357b1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1321,7 +1321,6 @@ struct drm_i915_private {
u32 pipestat_irq_mask[I915_MAX_PIPES];
struc
[Intel-gfx] [PATCH 3/4] drm: Introduce drm_fb_helper_prepare()
From: Thierry Reding
To implement hotplug detection in a race-free manner, drivers must call
drm_kms_helper_poll_init() before hotplug events can be triggered. Such
events can be triggered right after any of the encoders or connectors
are initialized. At the same time, if the drm_fb_helper_hotplug_event()
helper is used by a driver, then the poll helper requires some parts of
the FB helper to be initialized to prevent a crash.
At the same time, drm_fb_helper_init() requires information that is not
necessarily available at such an early stage (number of CRTCs and
connectors), so it cannot be used yet.
Add a new helper, drm_fb_helper_prepare(), that initializes the bare
minimum needed to allow drm_kms_helper_poll_init() to execute and any
subsequent hotplug events to be processed properly.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/armada/armada_fbdev.c | 2 +-
drivers/gpu/drm/ast/ast_fb.c | 4 ++-
drivers/gpu/drm/bochs/bochs_fbdev.c | 3 ++-
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 4 ++-
drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
drivers/gpu/drm/drm_fb_helper.c | 43 ---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 ++-
drivers/gpu/drm/gma500/framebuffer.c | 3 ++-
drivers/gpu/drm/i915/intel_fbdev.c| 3 ++-
drivers/gpu/drm/mgag200/mgag200_fb.c | 3 ++-
drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 ++-
drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
drivers/gpu/drm/qxl/qxl_fb.c | 5 +++-
drivers/gpu/drm/radeon/radeon_fb.c| 4 ++-
drivers/gpu/drm/tegra/fb.c| 4 +--
drivers/gpu/drm/udl/udl_fb.c | 3 ++-
include/drm/drm_fb_helper.h | 2 ++
18 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c
b/drivers/gpu/drm/armada/armada_fbdev.c
index 21aa1261dba2..9437e11d5df1 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -149,7 +149,7 @@ int armada_fbdev_init(struct drm_device *dev)
priv->fbdev = fbh;
- fbh->funcs = &armada_fb_helper_funcs;
+ drm_fb_helper_prepare(dev, fbh, &armada_fb_helper_funcs);
ret = drm_fb_helper_init(dev, fbh, 1, 1);
if (ret) {
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 2113894e4ff8..cba45c774552 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -328,8 +328,10 @@ int ast_fbdev_init(struct drm_device *dev)
return -ENOMEM;
ast->fbdev = afbdev;
- afbdev->helper.funcs = &ast_fb_helper_funcs;
spin_lock_init(&afbdev->dirty_lock);
+
+ drm_fb_helper_prepare(dev, &afbdev->helper, &ast_fb_helper_funcs);
+
ret = drm_fb_helper_init(dev, &afbdev->helper,
1, 1);
if (ret) {
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 17e5c17f2730..19cf3e9413b6 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -189,7 +189,8 @@ int bochs_fbdev_init(struct bochs_device *bochs)
{
int ret;
- bochs->fb.helper.funcs = &bochs_fb_helper_funcs;
+ drm_fb_helper_prepare(bochs->dev, &bochs->fb.helper,
+ &bochs_fb_helper_funcs);
ret = drm_fb_helper_init(bochs->dev, &bochs->fb.helper,
1, 1);
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 2bd0291168e4..2a135f253e29 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -306,9 +306,11 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
return -ENOMEM;
cdev->mode_info.gfbdev = gfbdev;
- gfbdev->helper.funcs = &cirrus_fb_helper_funcs;
spin_lock_init(&gfbdev->dirty_lock);
+ drm_fb_helper_prepare(cdev->dev, &gfbdev->helper,
+ &cirrus_fb_helper_funcs);
+
ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
cdev->num_crtc, CIRRUSFB_CONN_LIMIT);
if (ret) {
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c
index b74f9e58b69d..acbbd230e081 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -354,9 +354,10 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device
*dev,
return ERR_PTR(-ENOMEM);
}
- fbdev_cma->fb_helper.funcs = &drm_fb_cma_helper_funcs;
helper = &fbdev_cma->fb_helper;
+ drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs);
+
ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count);
if (ret < 0) {
dev_err(dev->dev, "Failed to initialize drm fb helper.\n");
diff --git a/drivers/gpu/
Re: [Intel-gfx] [PATCH 17/48] drm/i915: Track which ring a context ran on
On Fri, Apr 18, 2014 at 10:51:46AM +0100, Chris Wilson wrote: > On Fri, Dec 06, 2013 at 02:11:02PM -0800, Ben Widawsky wrote: > > From: Ben Widawsky > > > > Previously we dropped the association of a context to a ring. It is > > however very important to know which ring a context ran on (we could > > have reused the other member, but I was nitpicky). > > > > This is very important when we switch address spaces, which unlike > > context objects, do change per ring. > > > > As an example, if we have: > > > > RCS BCS > > ctxA > > ctx A > > ctx B > > ctxB > > > > Without tracking the last ring B ran on, we wouldn't know to switch the > > address space on BCS in the last row. > > > > As a result, we no longer need to track which ring a context "belongs" > > to, as it never really made much sense anyway. > > What about ring->last_context != to? That would force the update on BCS > from A to B. Yeah ctx->last_ring isn't really what we want, I'll add another JIRA for switching all the ctx->last_ring checks to instead check ring->last_context != to. Since that's what matters I think this doesn't have the possibility to have broken anything yet since we don't allow the same context on multiple rings. Except the default one, but mesa creates new contexts anyway afaik. Or am I wrong? It's a definite blocker for execlists and full ppgtt though. -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] Design review request: DRM color manager
On Tue, Apr 22, 2014 at 12:07:41PM +, Sharma, Shashank wrote: > Thanks again David, > Comments inline. Three things: - Please don't send out .pptx files to upstream/public mailing lists, that's just not how the upstream community works. - Please either fix up ms outlook to do proper in-line quoting or switch to a proper mail client for discussions on dri-devel. I'm ok with this on intel-gfx to some extend since that's our own turf, but on dri-devel the usual rules apply. - I think we should discuss this internally first or at least just on intel-gfx. David, thanks for taking a look at this but imo this shouldn't have escaped yet to the public. My apologies for wasting your time trying to review this proposal. Thanks, Daniel > > Regards > Shashank > -Original Message- > From: David Herrmann [mailto:dh.herrm...@gmail.com] > Sent: Tuesday, April 22, 2014 5:10 PM > To: Sharma, Shashank > Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Ville > Syrjälä; Thierry Reding; Alex Deucher; Sean Paul; robdcl...@gmail.com; > Mukherjee, Indranil; Jindal, Sonika; Korjani, Vikas; Shankar, Uma; Cn, > Ramakrishnan > Subject: Re: Design review request: DRM color manager > > Hi > > On Tue, Apr 22, 2014 at 12:21 PM, Sharma, Shashank > wrote: > > 1) Why do you register only a single property? Why not register a separate > > property for each color-correction that is available? This way you can drop > > the property-id and use the high-level DRM-prop IDs/names. > >>> That’s the whole idea of color manager. If we keep on creating properties > >>> for each color correction, there would be a big list and a lot of > >>> properties will be exposed. Instead one common blob which can represent > >>> all the properties, correction values and identifiers. It would be easy > >>> to club with atomic modeset kind-of designs also I believe. > > Where is the difference? With one _well-defined_ property for each type we > simply move the identification one level up. With your approach you just move > the type-id one level down into the blob. > > Or in other words: Where is the difference between calling > SetProperty() n-times, or calling it once but with a parameter describing > n-properties? With atomic-modesetting we can set as many properties as we > want and make the kernel apply them atomically. > > >>> Actually we also do not want to populate the property space also, as if > >>> there are 10 color correction methods possible for a hardware, we might > >>> end up listing 10 properties. And there won't be common properties > >>> across all the hardwares also. For example, Hardware A can have > >>> properties X Y Z but Hardware B can have W X and Z. This will make the > >>> property space inconsistent. But if we provide one common interface which > >>> will cover for all the properties, for all the hardwares in a single > >>> blob. The driver will dynamically register its property, in its own > >>> preferred name. A get_prop() will always list down all the supported > >>> color property by this hardware and driver. > > > 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC > > and/or plane. Isn't that enough information for the driver? > >>> This is to make it HW agonist. Actually that's CRTC ID / Plane ID / PIPE > >>> ID / all together an identifier. For example if I want to set gamma > >>> correction for sprite planes only, not on primary plane or pipe level, on > >>> VLV, its possible. This gives me flexibility to mention fine-tuned > >>> correction even in a CRTC. The driver's .enable method can take decision > >>> on this identifier based on the hardware capabilities. > > Yeah, but I meant the drmModeObjectSetProperty() ioctl already tales a > CRTC/Plane/Connector ID. So why duplicate that information in the blob? And > more importantly, what happens if you call > drmModeObjectSetProperty() on a plane but specify a CRTC ID in the blob? > Seems weird to me to support such setups. > > >>> The design is to register color-manager as a CRTC property, to make it > >>> consistent, and then give the fine tuning via this identifier byte. > Else we have to keep track of this in userspace, that which property is valid > for which extent. For example, Hue and saturation correction, on VLV, can be > applied on Sprite planes only(not on primary plane). So we have to send a > plane as an object here. > Rather in color manager case, we will always send the CRTC as an object to > IOCTL, but will specify SPRITE_PLANE as identifier. Does this sound less > weird now :) ? > > > 3) Please document the payload for each of the properties you define. > > If the property is a blob, there is no reason to make the properties > > generic. User-space requires a common syntax across all drivers, otherwise, > > it cannot make use of generic properties and you should use > > driver-dependent properties instead. > >>> Can you please elabo
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Kick start the rings
> Subject: [Intel-gfx] [PATCH 6/6] drm/i915: Kick start the rings
>
> On g4x, we have an issue where the register write to setup the rings do not
> always take. However, it appears that the current check also passes only by
> chance, a second reading of the register returns a different broekn value -
> but
> the GPU appears to function. Based on that observation, lets try feeding a nop
> into the ring and seeing if it advances as part of our startup sanity checks.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index a8c73a0d935d..bc52645fa8d5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -514,10 +514,14 @@ static int init_ring_common(struct intel_ring_buffer
> *ring)
> ((ring->size - PAGE_SIZE) & RING_NR_PAGES)
> | RING_VALID);
>
> + iowrite32(MI_NOOP, ring->virtual_start + 0);
> + iowrite32(MI_NOOP, ring->virtual_start + 4);
> + ring->write_tail(ring, 8);
Maybe add a comment in the code about why we are doing this? (otherwise, it
looks a bit magical)
> /* If the head is still not zero, the ring is dead */
> if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 &&
>I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj)
> &&
> - (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
> + (I915_READ_HEAD(ring) & HEAD_ADDR) == 8, 50)) {
> DRM_ERROR("%s initialization failed "
> "ctl %08x (valid? %d) head %08x tail %08x start %08x
> [expected %08lx]\n",
> ring->name,
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Mark device as wedged if we fail to resume
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Chris Wilson
> Sent: Wednesday, April 09, 2014 9:20 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 4/6] drm/i915: Mark device as wedged if we fail to
> resume
>
> During module load, if we fail to initialise the rings, we abort the load
> reporting
> EIO. However during resume, even though we report EIO as we fail to
> reinitialize the ringbuffers, the resume continues and the device is restored
> -
> albeit in a non-functional state. As we cannot execute any commands on the
> GPU, it is effectively wedged, mark it so.
>
> As we now preserve the ringbuffers across resume, this should prevent UXA
> from falling into the trap of repeatedly sending invalid batchbuffers and
> dropping all further rendering into /dev/null.
>
> Reported-and-tested-by: Jiri Kosina
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index 5d8250f7145d..629f8164a547 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -569,8 +569,10 @@ static int __i915_drm_thaw(struct drm_device *dev,
> bool restore_gtt_mappings)
> drm_mode_config_reset(dev);
>
> mutex_lock(&dev->struct_mutex);
> -
> - error = i915_gem_init_hw(dev);
> + if (i915_gem_init_hw(dev)) {
> + DRM_ERROR("failed to re-initialize GPU, declaring
> wedged!\n");
The "int error = 0;" is not needed anymore. Other than that:
Reviewed-by: Oscar Mateo
> + atomic_set_mask(I915_WEDGED, &dev_priv-
> >gpu_error.reset_counter);
> + }
> mutex_unlock(&dev->struct_mutex);
>
> /* We need working interrupts for modeset enabling ... */ @@
> -613,7 +615,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
> restore_gtt_mappings)
> mutex_unlock(&dev_priv->modeset_restore_lock);
>
> intel_runtime_pm_put(dev_priv);
> - return error;
> + return 0;
> }
>
> static int i915_drm_thaw(struct drm_device *dev)
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Include a little more information about why ring init fails
> Subject: [Intel-gfx] [PATCH 5/6] drm/i915: Include a little more information > about why ring init fails > > If we include the expected values for the failing ring register checks, it > makes it > marginally easier to see which is the culprit. > > Signed-off-by: Chris Wilson Reviewed-by: Oscar Mateo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Preserve ring buffers objects across resume
> Subject: [Intel-gfx] [PATCH 2/6] drm/i915: Preserve ring buffers objects
> across
> resume
>
> Tearing down the ring buffers across resume is overkill, risks unnecessary
> failure and increases fragmentation.
>
> After failure, since the device is still active we may end up trying to write
> into
> the dangling iomapping and trigger an oops.
>
> v2: stop_ringbuffers() was meant to call stop(ring) not
> cleanup(ring) during resume!
>
> Reported-by: Jae-hyeon Park
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=72351
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem.c | 20 +++-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 168 +--
> -
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> 3 files changed, 105 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index 6370a761d137..89e736c00ddc
> 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2383,6 +2383,11 @@ static void i915_gem_reset_ring_cleanup(struct
> drm_i915_private *dev_priv,
>
> i915_gem_free_request(request);
> }
> +
> + /* These may not have been flush before the reset, do so now */
> + kfree(ring->preallocated_lazy_request);
> + ring->preallocated_lazy_request = NULL;
> + ring->outstanding_lazy_seqno = 0;
> }
>
> void i915_gem_restore_fences(struct drm_device *dev) @@ -2423,8 +2428,6
> @@ void i915_gem_reset(struct drm_device *dev)
> for_each_ring(ring, dev_priv, i)
> i915_gem_reset_ring_cleanup(dev_priv, ring);
>
> - i915_gem_cleanup_ringbuffer(dev);
> -
> i915_gem_context_reset(dev);
>
> i915_gem_restore_fences(dev);
> @@ -4232,6 +4235,17 @@ void i915_gem_vma_destroy(struct i915_vma
> *vma)
> kfree(vma);
> }
>
> +static void
> +i915_gem_stop_ringbuffers(struct drm_device *dev) {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_ring_buffer *ring;
> + int i;
> +
> + for_each_ring(ring, dev_priv, i)
> + intel_stop_ring_buffer(ring);
> +}
> +
> int
> i915_gem_suspend(struct drm_device *dev) { @@ -4253,7 +4267,7 @@
> i915_gem_suspend(struct drm_device *dev)
> i915_gem_evict_everything(dev);
>
> i915_kernel_lost_context(dev);
> - i915_gem_cleanup_ringbuffer(dev);
> + i915_gem_stop_ringbuffers(dev);
>
> /* Hack! Don't let anybody do execbuf while we don't control the chip.
>* We need to replace this with a semaphore, or something.
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 9bed8764a62a..cce09f5a4426 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1301,45 +1301,39 @@ static void cleanup_status_page(struct
> intel_ring_buffer *ring)
>
> static int init_status_page(struct intel_ring_buffer *ring) {
> - struct drm_device *dev = ring->dev;
> struct drm_i915_gem_object *obj;
> - int ret;
>
> - obj = i915_gem_alloc_object(dev, 4096);
> - if (obj == NULL) {
> - DRM_ERROR("Failed to allocate status page\n");
> - ret = -ENOMEM;
> - goto err;
> - }
> + if ((obj = ring->status_page.obj) == NULL) {
> + int ret;
>
> - ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
> - if (ret)
> - goto err_unref;
> + obj = i915_gem_alloc_object(ring->dev, 4096);
> + if (obj == NULL) {
> + DRM_ERROR("Failed to allocate status page\n");
> + return -ENOMEM;
> + }
>
> - ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
> - if (ret)
> - goto err_unref;
> + ret = i915_gem_object_set_cache_level(obj,
> I915_CACHE_LLC);
> + if (ret)
> + goto err_unref;
> +
> + ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
> + if (ret) {
> +err_unref:
> + drm_gem_object_unreference(&obj->base);
> + return ret;
> + }
> +
> + ring->status_page.obj = obj;
> + }
>
> ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(obj);
> ring->status_page.page_addr = kmap(sg_page(obj->pages->sgl));
> - if (ring->status_page.page_addr == NULL) {
> - ret = -ENOMEM;
> - goto err_unpin;
> - }
> - ring->status_page.obj = obj;
> memset(ring->status_page.page_addr, 0, PAGE_SIZE);
>
> DRM_DEBUG_DRIVER("%s hws offset: 0x%08x\n",
> ring->name, ring->status_page.gfx_addr);
>
> return 0;
> -
> -err_unpin:
> - i915_gem_object_ggtt_unpin(obj);
> -err_unref:
> - drm_gem_object_unreference(&obj->base);
> -err:
> - return ret;
> }
>
> static int init_
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Allow the module to load even if we fail to setup rings
> Subject: [Intel-gfx] [PATCH 3/6] drm/i915: Allow the module to load even if we > fail to setup rings > > Even without enabling the ringbuffers to allow command execution, we can still > control the display engines to enable modesetting. So make the ringbuffer > initialization failure soft, and mark the GPU as wedged instead. > > v2: Only treat an EIO from ring initialisation as a soft failure, and abort > module > load for any other failure, such as allocation failures. > > v3: Add an *ERROR* prior to declaring the GPU wedged so that it stands out > like a sore thumb in the logs > > Signed-off-by: Chris Wilson > Reviewed-by: Jesse Barnes Reviewed-by: Oscar Mateo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Intel-specific primary plane handling (v4)
On Fri, Apr 11, 2014 at 02:13:42PM -0700, Matt Roper wrote:
> Intel hardware allows the primary plane to be disabled independently of
> the CRTC. Provide custom primary plane handling to allow this.
>
> v4:
> - Don't add a primary_plane field to intel_crtc; that was left over
>from a much earlier iteration of this patch series, but is no longer
>needed/used now that the DRM core primary plane support has been
>merged.
> v3:
> - Provide gen-specific primary plane format lists (suggested by Daniel
>Vetter).
> - If the primary plane is already enabled, go ahead and just call the
>primary plane helper to do the update (suggested by Daniel Vetter).
> - Don't try to disable the primary plane on destruction; the DRM layer
>should have already taken care of this for us.
> v2:
> - Unpin fb properly on primary plane disable
> - Provide an Intel-specific set of primary plane formats
> - Additional sanity checks on setplane (in line with the checks
>currently being done by the DRM core primary plane helper)
>
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/intel_display.c | 197
> ++-
> 1 file changed, 195 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1390ab5..3e4d36a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -39,8 +39,35 @@
> #include "i915_trace.h"
> #include
> #include
> +#include
> +#include
> #include
>
> +/* Primary plane formats supported by all gen */
> +#define COMMON_PRIMARY_FORMATS \
> + DRM_FORMAT_C8, \
> + DRM_FORMAT_RGB565, \
> + DRM_FORMAT_XRGB, \
> + DRM_FORMAT_ARGB
> +
> +/* Primary plane formats for gen <= 3 */
> +const static uint32_t intel_primary_formats_gen3[] = {q
'static const' is the more typical order
> + COMMON_PRIMARY_FORMATS,
> + DRM_FORMAT_XRGB1555,
> + DRM_FORMAT_ARGB1555,
> +};
> +
> +/* Primary plane formats for gen >= 4 */
> +const static uint32_t intel_primary_formats_gen4[] = {
ditto
> + COMMON_PRIMARY_FORMATS, \
> + DRM_FORMAT_XBGR,
> + DRM_FORMAT_ABGR,
> + DRM_FORMAT_XRGB2101010,
> + DRM_FORMAT_ARGB2101010,
> + DRM_FORMAT_XBGR2101010,
> + DRM_FORMAT_ABGR2101010,
> +};
> +
> static void intel_increase_pllclock(struct drm_crtc *crtc);
> static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
>
> @@ -10556,17 +10583,183 @@ static void intel_shared_dpll_init(struct
> drm_device *dev)
> BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
> }
>
> +static int
> +intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
> + struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_framebuffer *tmpfb;
> + struct drm_rect dest = {
> + .x1 = crtc_x,
> + .y1 = crtc_y,
> + .x2 = crtc_x + crtc_w,
> + .y2 = crtc_y + crtc_h,
> + };
> + struct drm_rect clip = {
> + .x2 = crtc->mode.hdisplay,
> + .y2 = crtc->mode.vdisplay,
> + };
'clip' can be const, and these should be pipe_src_{w,h} just like
we have in the sprite code.
> + int ret;
> +
> + /*
> + * At the moment we use the same set of setplane restrictions as the
> + * DRM primary plane helper, so go ahead and just call the helper if
> + * the primary plane is already enabled. We only need to take special
> + * action if the primary plane is disabled (something i915 can do but
> + * the generic helper can't).
> + */
> + if (intel_crtc->primary_enabled)
> + return drm_primary_helper_update(plane, crtc, fb,
> + crtc_x, crtc_y,
> + crtc_w, crtc_h,
> + src_x, src_y,
> + src_w, src_h);
Why would we want to call that if we have a custom implementation
anyway?
> +
> + /* setplane API takes shifted source rectangle values; unshift them */
> + src_x >>= 16;
> + src_y >>= 16;
> + src_w >>= 16;
> + src_h >>= 16;
> +
> + /*
> + * Current hardware can't reposition the primary plane or scale it
> + * (although this could change in the future).
> + */
> + drm_rect_intersect(&dest, &clip);
src needs to be clipped too. I guess we should get a sufficiently good
result if we just call 'drm_rect_clip_scaled(..., 1, 1)' here. Ie.
clip after throwing
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Replace hardcoded cacheline size with macro
> Subject: [Intel-gfx] [PATCH 1/6] drm/i915: Replace hardcoded cacheline size > with macro > > For readibility and guess at the meaning behind the constants. > > v2: Claim only the meagerest connections with reality. > > Signed-off-by: Chris Wilson Reviewed-by: Oscar Mateo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Design review request: DRM color manager
Thanks again David, Comments inline. Regards Shashank -Original Message- From: David Herrmann [mailto:dh.herrm...@gmail.com] Sent: Tuesday, April 22, 2014 5:10 PM To: Sharma, Shashank Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Ville Syrjälä; Thierry Reding; Alex Deucher; Sean Paul; robdcl...@gmail.com; Mukherjee, Indranil; Jindal, Sonika; Korjani, Vikas; Shankar, Uma; Cn, Ramakrishnan Subject: Re: Design review request: DRM color manager Hi On Tue, Apr 22, 2014 at 12:21 PM, Sharma, Shashank wrote: > 1) Why do you register only a single property? Why not register a separate > property for each color-correction that is available? This way you can drop > the property-id and use the high-level DRM-prop IDs/names. >>> That’s the whole idea of color manager. If we keep on creating properties >>> for each color correction, there would be a big list and a lot of >>> properties will be exposed. Instead one common blob which can represent all >>> the properties, correction values and identifiers. It would be easy to club >>> with atomic modeset kind-of designs also I believe. Where is the difference? With one _well-defined_ property for each type we simply move the identification one level up. With your approach you just move the type-id one level down into the blob. Or in other words: Where is the difference between calling SetProperty() n-times, or calling it once but with a parameter describing n-properties? With atomic-modesetting we can set as many properties as we want and make the kernel apply them atomically. >>> Actually we also do not want to populate the property space also, as if >>> there are 10 color correction methods possible for a hardware, we might end >>> up listing 10 properties. And there won't be common properties across all >>> the hardwares also. For example, Hardware A can have properties X Y Z but >>> Hardware B can have W X and Z. This will make the property space >>> inconsistent. But if we provide one common interface which will cover for >>> all the properties, for all the hardwares in a single blob. The driver will >>> dynamically register its property, in its own preferred name. A get_prop() >>> will always list down all the supported color property by this hardware and >>> driver. > 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC > and/or plane. Isn't that enough information for the driver? >>> This is to make it HW agonist. Actually that's CRTC ID / Plane ID / PIPE ID >>> / all together an identifier. For example if I want to set gamma correction >>> for sprite planes only, not on primary plane or pipe level, on VLV, its >>> possible. This gives me flexibility to mention fine-tuned correction even >>> in a CRTC. The driver's .enable method can take decision on this >>> identifier based on the hardware capabilities. Yeah, but I meant the drmModeObjectSetProperty() ioctl already tales a CRTC/Plane/Connector ID. So why duplicate that information in the blob? And more importantly, what happens if you call drmModeObjectSetProperty() on a plane but specify a CRTC ID in the blob? Seems weird to me to support such setups. >>> The design is to register color-manager as a CRTC property, to make it >>> consistent, and then give the fine tuning via this identifier byte. Else we have to keep track of this in userspace, that which property is valid for which extent. For example, Hue and saturation correction, on VLV, can be applied on Sprite planes only(not on primary plane). So we have to send a plane as an object here. Rather in color manager case, we will always send the CRTC as an object to IOCTL, but will specify SPRITE_PLANE as identifier. Does this sound less weird now :) ? > 3) Please document the payload for each of the properties you define. > If the property is a blob, there is no reason to make the properties generic. > User-space requires a common syntax across all drivers, otherwise, it cannot > make use of generic properties and you should use driver-dependent properties > instead. >>> Can you please elaborate a bit more ? I believe that a blob is a superset >>> of single and multi-valued properties. So we can use the byte defined for >>> and specify both single value and multi value >>> correction using the same interface, >> method and protocol. So any >>> userspace can just follow this, any can give commands to any driver. Well, your document doesn't describe the payload at all. I just wanted a description of what kind of information is expected. Number of arguments, argument size, argument types, argument description.. and so on. Sure, I will further document it very clearly about arguments and descriptions. Actually we have discussed the protocol in the color EDID section, which tells us about the 4 byte protocol and expectation, but that’s elementary. > 4) We have a tuple-type for properties. So in case you only need 32b
Re: [Intel-gfx] [PATCH I-g-t V2 2/2] tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU<->GPU sync under multi BSD rings
On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> The Broadwell GT3 machine has two independent BSD rings in kernel driver while
> it is transparent to the user-space driver. In such case it needs to check
> the CPU<->GPU sync for the second BSD ring.
>
> V1->V2: Follow Daniel's comment to add one subtext instead of one individual
> test case, which is used to test the CPU<->GPU sync under multi BSD rings
>
> Signed-off-by: Zhao Yakui
> ---
> tests/gem_dummy_reloc_loop.c | 102
> +-
> 1 file changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
> index a61b59b..660d8e1 100644
> --- a/tests/gem_dummy_reloc_loop.c
> +++ b/tests/gem_dummy_reloc_loop.c
> @@ -48,6 +48,13 @@ static drm_intel_bufmgr *bufmgr;
> struct intel_batchbuffer *batch;
> static drm_intel_bo *target_buffer;
>
> +#define NUM_FD 50
> +
> +static int mfd[NUM_FD];
> +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> +static struct intel_batchbuffer *mbatch[NUM_FD];
> +static drm_intel_bo *mbuffer[NUM_FD];
> +
> /*
> * Testcase: Basic check of ring<->cpu sync using a dummy reloc
> *
> @@ -124,6 +131,50 @@ dummy_reloc_loop_random_ring(int num_rings)
> }
> }
>
> +static void
> +dummy_reloc_loop_random_ring_multi_fd(int num_rings)
> +{
> + int i;
> + struct intel_batchbuffer *saved_batch;
> +
> + saved_batch = batch;
> +
> + srandom(0xdeadbeef);
> +
> + for (i = 0; i < 0x10; i++) {
> + int mindex;
> + int ring = random() % num_rings + 1;
> +
> + mindex = random() % NUM_FD;
> + batch = mbatch[mindex];
> +
> + if (ring == I915_EXEC_RENDER) {
> + BEGIN_BATCH(4);
> + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> + OUT_BATCH(0x); /* compare dword */
> + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> + I915_GEM_DOMAIN_RENDER, 0);
> + OUT_BATCH(MI_NOOP);
> + ADVANCE_BATCH();
> + } else {
> + BEGIN_BATCH(4);
> + OUT_BATCH(MI_FLUSH_DW | 1);
> + OUT_BATCH(0); /* reserved */
> + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> + I915_GEM_DOMAIN_RENDER, 0);
> + OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
> + ADVANCE_BATCH();
> + }
> + intel_batchbuffer_flush_on_ring(batch, ring);
> +
> + drm_intel_bo_map(target_buffer, 0);
> + // map to force waiting on rendering
> + drm_intel_bo_unmap(target_buffer);
> + }
> +
> + batch = saved_batch;
> +}
> +
> int fd;
> int devid;
> int num_rings;
> @@ -133,6 +184,7 @@ igt_main
> igt_skip_on_simulation();
>
> igt_fixture {
> + int i;
> fd = drm_open_any();
> devid = intel_get_drm_devid(fd);
> num_rings = gem_get_num_rings(fd);
> @@ -148,6 +200,35 @@ igt_main
>
> target_buffer = drm_intel_bo_alloc(bufmgr, "target bo", 4096,
> 4096);
> igt_assert(target_buffer);
> +
> + /* Create multi drm_fd and map one gem object to multi
> gem_contexts */
> + {
> + unsigned int target_flink;
> + char buffer_name[32];
> + if (dri_bo_flink(target_buffer, &target_flink)) {
> + printf("fail to get flink for target buffer\n");
> + igt_assert(0);
For the future: could be just igt_assert_f().
> + }
> + for (i = 0; i < NUM_FD; i++) {
> + mfd[i] = 0;
> + mbufmgr[i] = NULL;
> + mbuffer[i] = NULL;
> + }
Nitpick: the above are all statics, so no need to init them.
Other than the above this looks good:
Reviewed-by: Imre Deak
> + for (i = 0; i < NUM_FD; i++) {
> + sprintf(buffer_name, "Target buffer %d\n", i);
> + mfd[i] = drm_open_any();
> + mbufmgr[i] = drm_intel_bufmgr_gem_init(mfd[i],
> 4096);
> + igt_assert(mbufmgr[i]);
> + drm_intel_bufmgr_gem_enable_reuse(mbufmgr[i]);
> + mbatch[i] = intel_batchbuffer_alloc(mbufmgr[i],
> devid);
> + igt_assert(mbufmgr[i]);
> + mbuffer[i] = intel_bo_gem_create_from_name(
> + mbufmgr[i],
> + buffer_name,
> +
Re: [Intel-gfx] [Bug 3.15-rc2] intel framebuffer broken
On Mon, 21 Apr 2014, Knut Petersen wrote: > Booting kernel 3.15-rc2 on an AOpen i915GMm-hfs, I see the framebuffer broken. > Only about the upper left quarter of the monitor is used for displaying the > boot messages, > these (and the cursor) are replicated at the right of that area. Is this before or after i915.ko is loaded? > Approximately the lower half of the monitor stays black. The output of the > xserver > is not affected. Most likely the fastest way to root cause is to bisect, please do. To keep track of all the bugs we have, please file a bug on DRM/Intel at [1] and while at it attach dmesg with drm.debug=0xe from the boot. BR, Jani. [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH I-g-t V2 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
> The Broadwell GT3 machine has two independent BSD rings in kernel driver while
> it is transparent to the user-space driver. In such case it needs to check
> the ring sync between the two BSD rings. At the same time it also needs to
> check the sync among the second BSD ring and the other rings.
>
> Signed-off-by: Zhao Yakui
> ---
> tests/Makefile.sources |1 +
> tests/gem_multi_bsd_sync_loop.c | 172
> +++
> 2 files changed, 173 insertions(+)
> create mode 100644 tests/gem_multi_bsd_sync_loop.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c957ace..7cd9ca8 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -105,6 +105,7 @@ TESTS_progs = \
> gem_render_tiled_blits \
> gem_ring_sync_copy \
> gem_ring_sync_loop \
> + gem_multi_bsd_sync_loop \
> gem_seqno_wrap \
> gem_set_tiling_vs_gtt \
> gem_set_tiling_vs_pwrite \
> diff --git a/tests/gem_multi_bsd_sync_loop.c b/tests/gem_multi_bsd_sync_loop.c
> new file mode 100644
> index 000..7f5b832
> --- /dev/null
> +++ b/tests/gem_multi_bsd_sync_loop.c
> @@ -0,0 +1,172 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Daniel Vetter (based on
> gem_ring_sync_loop_*.c)
> + *Zhao Yakui
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "drm.h"
> +#include "ioctl_wrappers.h"
> +#include "drmtest.h"
> +#include "intel_bufmgr.h"
> +#include "intel_batchbuffer.h"
> +#include "intel_io.h"
> +#include "i830_reg.h"
> +#include "intel_chipset.h"
> +
> +static drm_intel_bufmgr *bufmgr;
> +struct intel_batchbuffer *batch;
> +static drm_intel_bo *target_buffer;
> +
> +#define NUM_FD 50
> +
> +static int mfd[NUM_FD];
> +static drm_intel_bufmgr *mbufmgr[NUM_FD];
> +static struct intel_batchbuffer *mbatch[NUM_FD];
> +static drm_intel_bo *mbuffer[NUM_FD];
> +
> +
> +/*
> + * Testcase: Basic check of ring<->ring sync using a dummy reloc
> + *
> + * Extremely efficient at catching missed irqs with semaphores=0 ...
> + */
> +
> +#define MI_COND_BATCH_BUFFER_END (0x36<<23 | 1)
> +#define MI_DO_COMPARE(1<<21)
> +
> +static void
> +store_dword_loop(int fd)
> +{
> + int i;
> + int num_rings = gem_get_num_rings(fd);
> +
> + srandom(0xdeadbeef);
> +
> + for (i = 0; i < SLOW_QUICK(0x10, 10); i++) {
> + int ring, mindex;
> + ring = random() % num_rings + 1;
> + mindex = random() % NUM_FD;
> + batch = mbatch[mindex];
> + if (ring == I915_EXEC_RENDER) {
> + BEGIN_BATCH(4);
> + OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
> + OUT_BATCH(0x); /* compare dword */
> + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> + I915_GEM_DOMAIN_RENDER, 0);
> + OUT_BATCH(MI_NOOP);
> + ADVANCE_BATCH();
> + } else {
> + BEGIN_BATCH(4);
> + OUT_BATCH(MI_FLUSH_DW | 1);
> + OUT_BATCH(0); /* reserved */
> + OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
> + I915_GEM_DOMAIN_RENDER, 0);
> + OUT_BATCH(MI_NOOP | (1<<22) | (0xf));
> + ADVANCE_BATCH();
> + }
> + intel_batchbuffer_flush_on_ring(batch, ring);
> + }
> +
> + drm_intel_bo_map(target_buffer, 0);
> + // map to force waiting on rendering
> + drm_intel_bo_unmap(target_buffer);
This test looks the same as dummy_
Re: [Intel-gfx] [PATCH] drm/fb-helper: Fix hpd vs. initial config races
On Tue, Apr 22, 2014 at 12:08 PM, Thierry Reding
wrote:
> void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper
> *helper,
>const struct drm_fb_helper_funcs *funcs)
> {
> helper->funcs = funcs;
> helper->dev = dev;
> }
>
> So I wonder if that's still what we want or whether drivers should
> simply be doing that manually if they need to. Having a function for it
> gives us a place to document things, though, and perhaps at some point
> we'll have to extend this, so it may be a good idea after all, even if
> it's just the two lines currently.
Yeah the usefulness of this will be in the documentation that explains
how to use it, not in the code sharing/extraction. For critical code
we could just plunk it into a static inline, but since this is init
code I wouldn't really care with that.
-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] Design review request: DRM color manager
Hi On Tue, Apr 22, 2014 at 12:21 PM, Sharma, Shashank wrote: > 1) Why do you register only a single property? Why not register a separate > property for each color-correction that is available? This way you can drop > the property-id and use the high-level DRM-prop IDs/names. >>> That’s the whole idea of color manager. If we keep on creating properties >>> for each color correction, there would be a big list and a lot of >>> properties will be exposed. Instead one common blob which can represent all >>> the properties, correction values and identifiers. It would be easy to club >>> with atomic modeset kind-of designs also I believe. Where is the difference? With one _well-defined_ property for each type we simply move the identification one level up. With your approach you just move the type-id one level down into the blob. Or in other words: Where is the difference between calling SetProperty() n-times, or calling it once but with a parameter describing n-properties? With atomic-modesetting we can set as many properties as we want and make the kernel apply them atomically. > 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC > and/or plane. Isn't that enough information for the driver? >>> This is to make it HW agonist. Actually that's CRTC ID / Plane ID / PIPE ID >>> / all together an identifier. For example if I want to set gamma correction >>> for sprite planes only, not on primary plane or pipe level, on VLV, its >>> possible. This gives me flexibility to mention fine-tuned correction even >>> in a CRTC. The driver's .enable method can take decision on this >>> identifier based on the hardware capabilities. Yeah, but I meant the drmModeObjectSetProperty() ioctl already tales a CRTC/Plane/Connector ID. So why duplicate that information in the blob? And more importantly, what happens if you call drmModeObjectSetProperty() on a plane but specify a CRTC ID in the blob? Seems weird to me to support such setups. > 3) Please document the payload for each of the properties you define. > If the property is a blob, there is no reason to make the properties generic. > User-space requires a common syntax across all drivers, otherwise, it cannot > make use of generic properties and you should use driver-dependent properties > instead. >>> Can you please elaborate a bit more ? I believe that a blob is a superset >>> of single and multi-valued properties. So we can use the byte defined for >>> and specify both single value and multi value >>> correction using the same interface, >> method and protocol. So any >>> userspace can just follow this, any can give commands to any driver. Well, your document doesn't describe the payload at all. I just wanted a description of what kind of information is expected. Number of arguments, argument size, argument types, argument description.. and so on. > 4) We have a tuple-type for properties. So in case you only need 32bit > payloads for a given property, you can combine enable/disable and value in a > single 64bit property. >>> But properties like CSC and Gamma correction need multiple correction >>> values, up to 256 32-bit values. For this we need more no of values. AM I >>> getting it right ? Sure. Thanks David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Design review request: DRM color manager
Thanks for the review and comments David. Please find my comments inline. Regards Shashank -Original Message- From: David Herrmann [mailto:dh.herrm...@gmail.com] Sent: Tuesday, April 22, 2014 3:08 PM To: Sharma, Shashank Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Ville Syrjälä; Thierry Reding; Alex Deucher; Sean Paul; robdcl...@gmail.com; Mukherjee, Indranil; Jindal, Sonika; Korjani, Vikas; Shankar, Uma; Cn, Ramakrishnan Subject: Re: Design review request: DRM color manager Hi On Tue, Apr 22, 2014 at 6:11 AM, Sharma, Shashank wrote: > Gentle reminder Usual approach is to send any proposals as inline plain-text. It's really hard to comment on attachments, especially if it's an MS-office format. Anyhow, some comments on the proposal: >> Sorry, I found it difficult to share that design on text only style. I will >> keep this in mind. 1) Why do you register only a single property? Why not register a separate property for each color-correction that is available? This way you can drop the property-id and use the high-level DRM-prop IDs/names. >> That’s the whole idea of color manager. If we keep on creating properties >> for each color correction, there would be a big list and a lot of properties >> will be exposed. Instead one common blob which can represent all the >> properties, correction values and identifiers. It would be easy to club with >> atomic modeset kind-of designs also I believe. 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC and/or plane. Isn't that enough information for the driver? >> This is to make it HW agonist. Actually that's CRTC ID / Plane ID / PIPE ID >> / all together an identifier. For example if I want to set gamma correction >> for sprite planes only, not on primary plane or pipe level, on VLV, its >> possible. This gives me flexibility to mention fine-tuned correction even in >> a CRTC. The driver's .enable method can take decision on this identifier >> based on the hardware capabilities. 3) Please document the payload for each of the properties you define. If the property is a blob, there is no reason to make the properties generic. User-space requires a common syntax across all drivers, otherwise, it cannot make use of generic properties and you should use driver-dependent properties instead. >> Can you please elaborate a bit more ? I believe that a blob is a superset of >> single and multi-valued properties. So we can use the byte defined for > of correction bytes> and specify both single value and multi value >> correction using the same interface, >> method and protocol. So any >> userspace can just follow this, any can give commands to any driver. 4) We have a tuple-type for properties. So in case you only need 32bit payloads for a given property, you can combine enable/disable and value in a single 64bit property. >> But properties like CSC and Gamma correction need multiple correction >> values, up to 256 32-bit values. For this we need more no of values. AM I >> getting it right ? 5) Please use common prefixes to group related functions: Use drm_color_manager_register() instead of drm_register_color_manager(). Similarly, use drm_color_manager_set_property() instead of drm_set_color_manager_property().. >> Sure, I will do so. Thanks David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/fb-helper: Fix hpd vs. initial config races
On Tue, Apr 22, 2014 at 10:26:37AM +0200, Daniel Vetter wrote:
> On Thu, Apr 17, 2014 at 10:38:17AM +0200, Thierry Reding wrote:
> > On Wed, Apr 16, 2014 at 04:45:21PM +0200, Daniel Vetter wrote:
> > > Some drivers need to be able to have a perfect race-free fbcon setup.
> > > Current drivers only enable hotplug processing after the call to
> > > drm_fb_helper_initial_config which leaves a tiny but important race.
> > >
> > > This race is especially noticable on embedded platforms where the
> > > driver itself enables the voltage for the hdmi output, since only then
> > > will monitors (after a bit of delay, as usual) respond by asserting
> > > the hpd pin.
> > >
> > > Most of the infrastructure is already there with the split-out
> > > drm_fb_helper_init. And drm_fb_helper_initial_config already has all
> > > the required locking to handle concurrent hpd events since
> > >
> > > commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f
> > > Author: Daniel Vetter
> > > Date: Thu Mar 20 14:26:35 2014 +0100
> > >
> > > drm/fb-helper: improve drm_fb_helper_initial_config locking
> > >
> > > The only missing bit is making drm_fb_helper_hotplug_event save
> > > against concurrent calls of drm_fb_helper_initial_config. The only
> > > unprotected bit is the check for fb_helper->fb.
> > >
> > > With that drivers can first initialize the fb helper, then enabel
> > > hotplug processing and then set up the initial config all in a
> > > completely race-free manner. Update kerneldoc and convert i915 as a
> > > proof of concept.
> > >
> > > Feature requested by Thierry since his tegra driver atm reliably boots
> > > slowly enough to misses the hotplug event for an external hdmi screen,
> > > but also reliably boots to quickly for the hpd pin to be asserted when
> > > the fb helper calls into the hdmi ->detect function.
> > >
> > > Cc: Thierry Reding
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 11 +--
> > > drivers/gpu/drm/i915/i915_dma.c | 3 ---
> > > drivers/gpu/drm/i915/i915_drv.c | 2 --
> > > drivers/gpu/drm/i915/i915_drv.h | 1 -
> > > drivers/gpu/drm/i915/i915_irq.c | 4
> > > 5 files changed, 5 insertions(+), 16 deletions(-)
> >
> > The FB helper parts:
> >
> > Tested-by: Thierry Reding
> >
> > But I have one comment below...
> >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > [...]
> > > - * Note that the driver must ensure that this is only called _after_ the
> > > fb has
> > > - * been fully set up, i.e. after the call to
> > > drm_fb_helper_initial_config.
> > > + * Note that drivers may call this even before calling
> > > + * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This
> > > allows
> >
> > I don't think the requirement is that strict. To elaborate: on Tegra we
> > cannot call drm_fb_helper_init() because the number of CRTCs and
> > connectors isn't known this early. We determine that dynamically after
> > all sub-devices have been initialized. So instead of calling
> > drm_fb_helper_init() before drm_kms_helper_poll_init(), I did something
> > more minimal (see attached patch). It's kind of difficult to tell
> > because of the context, but tegra_drm_fb_prepare() sets up the mode
> > config and functions and allocate memory for the FB helper and sets the
> > FB helper functions.
> >
> > This may not be enough for all drivers, but on Tegra the implementation
> > of .output_poll_changed() simply calls drm_fb_helper_hotplug_event(),
> > which will work fine with just that rudimentary initialization.
>
> Hm yeah I think this should be sufficient, too. It would be good to
> extract this minimal initialization into a new drm_fb_helper_prepare
> function and update the kerneldoc a bit more. Maybe as a patch on top of
> mine?
It would be essentially this:
void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
const struct drm_fb_helper_funcs *funcs)
{
helper->funcs = funcs;
helper->dev = dev;
}
So I wonder if that's still what we want or whether drivers should
simply be doing that manually if they need to. Having a function for it
gives us a place to document things, though, and perhaps at some point
we'll have to extend this, so it may be a good idea after all, even if
it's just the two lines currently.
Thierry
pgpQw6f4T3RMC.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Design review request: DRM color manager
Hi On Tue, Apr 22, 2014 at 6:11 AM, Sharma, Shashank wrote: > Gentle reminder Usual approach is to send any proposals as inline plain-text. It's really hard to comment on attachments, especially if it's an MS-office format. Anyhow, some comments on the proposal: 1) Why do you register only a single property? Why not register a separate property for each color-correction that is available? This way you can drop the property-id and use the high-level DRM-prop IDs/names. 2) What is the CRTC-ID for? DRM properties can be set on a specific CRTC and/or plane. Isn't that enough information for the driver? 3) Please document the payload for each of the properties you define. If the property is a blob, there is no reason to make the properties generic. User-space requires a common syntax across all drivers, otherwise, it cannot make use of generic properties and you should use driver-dependent properties instead. 4) We have a tuple-type for properties. So in case you only need 32bit payloads for a given property, you can combine enable/disable and value in a single 64bit property. 5) Please use common prefixes to group related functions: Use drm_color_manager_register() instead of drm_register_color_manager(). Similarly, use drm_color_manager_set_property() instead of drm_set_color_manager_property().. Thanks David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/fb-helper: Fix hpd vs. initial config races
On Tue, Apr 22, 2014 at 10:26:37AM +0200, Daniel Vetter wrote: > On Thu, Apr 17, 2014 at 10:38:17AM +0200, Thierry Reding wrote: > > On Wed, Apr 16, 2014 at 04:45:21PM +0200, Daniel Vetter wrote: > > > Some drivers need to be able to have a perfect race-free fbcon setup. > > > Current drivers only enable hotplug processing after the call to > > > drm_fb_helper_initial_config which leaves a tiny but important race. > > > > > > This race is especially noticable on embedded platforms where the > > > driver itself enables the voltage for the hdmi output, since only then > > > will monitors (after a bit of delay, as usual) respond by asserting > > > the hpd pin. > > > > > > Most of the infrastructure is already there with the split-out > > > drm_fb_helper_init. And drm_fb_helper_initial_config already has all > > > the required locking to handle concurrent hpd events since > > > > > > commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f > > > Author: Daniel Vetter > > > Date: Thu Mar 20 14:26:35 2014 +0100 > > > > > > drm/fb-helper: improve drm_fb_helper_initial_config locking > > > > > > The only missing bit is making drm_fb_helper_hotplug_event save > > > against concurrent calls of drm_fb_helper_initial_config. The only > > > unprotected bit is the check for fb_helper->fb. > > > > > > With that drivers can first initialize the fb helper, then enabel > > > hotplug processing and then set up the initial config all in a > > > completely race-free manner. Update kerneldoc and convert i915 as a > > > proof of concept. > > > > > > Feature requested by Thierry since his tegra driver atm reliably boots > > > slowly enough to misses the hotplug event for an external hdmi screen, > > > but also reliably boots to quickly for the hpd pin to be asserted when > > > the fb helper calls into the hdmi ->detect function. > > > > > > Cc: Thierry Reding > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/drm_fb_helper.c | 11 +-- > > > drivers/gpu/drm/i915/i915_dma.c | 3 --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > > drivers/gpu/drm/i915/i915_irq.c | 4 > > > 5 files changed, 5 insertions(+), 16 deletions(-) > > > > The FB helper parts: > > > > Tested-by: Thierry Reding > > > > But I have one comment below... > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > > b/drivers/gpu/drm/drm_fb_helper.c > > [...] > > > - * Note that the driver must ensure that this is only called _after_ the > > > fb has > > > - * been fully set up, i.e. after the call to > > > drm_fb_helper_initial_config. > > > + * Note that drivers may call this even before calling > > > + * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This > > > allows > > > > I don't think the requirement is that strict. To elaborate: on Tegra we > > cannot call drm_fb_helper_init() because the number of CRTCs and > > connectors isn't known this early. We determine that dynamically after > > all sub-devices have been initialized. So instead of calling > > drm_fb_helper_init() before drm_kms_helper_poll_init(), I did something > > more minimal (see attached patch). It's kind of difficult to tell > > because of the context, but tegra_drm_fb_prepare() sets up the mode > > config and functions and allocate memory for the FB helper and sets the > > FB helper functions. > > > > This may not be enough for all drivers, but on Tegra the implementation > > of .output_poll_changed() simply calls drm_fb_helper_hotplug_event(), > > which will work fine with just that rudimentary initialization. > > Hm yeah I think this should be sufficient, too. It would be good to > extract this minimal initialization into a new drm_fb_helper_prepare > function and update the kerneldoc a bit more. Maybe as a patch on top of > mine? > > Then we could merge this all as an early tegra-next pull to Dave. Sounds like a good idea. I'll go prepare a patch. Thierry pgpHKdmhxsNA0.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Warning on resume
On Sun, Apr 20, 2014 at 12:50:23PM -0700, Stephen Hemminger wrote: > I got this on my desktop (Haswell) box when resuming from suspend > with Debian testing kernel (3.13). I've just worked on a massive patch series to rework the haswell WRPLL. It kills the hsw specific shared pll framework and switches it to the commone framework used by other i915 platforms: http://cgit.freedesktop.org/~danvet/drm/log/?h=runtime-pm-for-dpms Can you please give it a spin? Thanks, Daniel > > > [147765.739493] [ cut here ] > [147765.739501] WARNING: CPU: 1 PID: 29426 at > /build/linux-oxWk_8/linux-3.13.7/drivers/gpu/drm/i915/intel_ddi.c:763 > intel_ddi_pll_mode_set+0x218/0x8e0 [i915]() > [147765.739502] WRPLL already enabled > [147765.739513] Modules linked in: nls_utf8 nls_cp437 vfat fat ipt_MASQUERADE > iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack > nf_conntrack ipt_REJECT xt_CHECKSUM iptable_mangle xt_tcpudp tun bridge stp > llc usb_storage ip6table_filter ip6_tables iptable_filter ip_tables > ebtable_nat ebtables x_tables bnep rfcomm bluetooth rfkill > cpufreq_conservative cpufreq_stats cpufreq_userspace cpufreq_powersave > binfmt_misc sch_fq_codel fuse loop parport_pc ppdev lp parport > snd_hda_codec_hdmi snd_hda_codec_realtek x86_pkg_temp_thermal > intel_powerclamp iTCO_wdt coretemp iTCO_vendor_support kvm_intel kvm > crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel evdev > aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper snd_seq_midi > snd_hda_intel snd_seq_midi_event cryptd snd_hda_codec psmouse snd_rawmidi > serio_raw snd_hwdep snd_pcm_oss snd_mixer_oss lpc_ich i915 mfd_core snd_seq > snd_pcm snd_seq_device snd_page_alloc i2c_i801 snd_timer drm_kms_helper snd > drm m ei_me video mei soundcore processor button ext4 crc16 mbcache jbd2 btrfs xor raid6_pq crc32c libcrc32c dm_mod sg sd_mod sr_mod crc_t10dif cdrom crct10dif_common hid_generic hid_belkin usbhid hid ahci libahci firewire_ohci libata scsi_mod firewire_core crc_itu_t igb ehci_pci xhci_hcd i2c_algo_bit ehci_hcd e1000e i2c_core fan usbcore thermal usb_common thermal_sys ixgbe dca ptp pps_core mdio > [147765.739526] CPU: 1 PID: 29426 Comm: kworker/u16:29 Tainted: GW > 3.13-1-amd64 #1 Debian 3.13.7-1 > [147765.739526] Hardware name: Supermicro X10SAE/X10SAE, BIOS 1.1a 01/03/2014 > [147765.739528] Workqueue: events_unbound async_run_entry_fn > [147765.739530] 0009 814a1327 8802ae387a88 > 8105ba72 > [147765.739531] 88040b23c000 8802ae387ad8 0001 > 000278d0 > [147765.739532] 880409928000 8105bad7 a0541431 > 88040018 > [147765.739532] Call Trace: > [147765.739534] [] ? dump_stack+0x41/0x51 > [147765.739535] [] ? warn_slowpath_common+0x72/0x90 > [147765.739536] [] ? warn_slowpath_fmt+0x47/0x50 > [147765.739542] [] ? gen6_read32+0x48/0x110 [i915] > [147765.739548] [] ? intel_ddi_pll_mode_set+0x218/0x8e0 > [i915] > [147765.739554] [] ? ironlake_enable_vblank+0x26/0x80 > [i915] > [147765.739560] [] ? haswell_crtc_mode_set+0x36/0x480 > [i915] > [147765.739566] [] ? __intel_set_mode+0x646/0x1500 [i915] > [147765.739571] [] ? gen6_read32+0x48/0x110 [i915] > [147765.739577] [] ? > intel_modeset_setup_hw_state+0xb19/0xc20 [i915] > [147765.739581] [] ? __i915_drm_thaw+0x14a/0x200 [i915] > [147765.739585] [] ? i915_resume+0x51/0x70 [i915] > [147765.739587] [] ? pci_pm_thaw+0x90/0x90 > [147765.739588] [] ? dpm_run_callback+0x2e/0x70 > [147765.739589] [] ? device_resume+0x8c/0x180 > [147765.739591] [] ? async_resume+0x14/0x40 > [147765.739592] [] ? async_run_entry_fn+0x2d/0x120 > [147765.739593] [] ? process_one_work+0x16d/0x420 > [147765.739594] [] ? worker_thread+0x116/0x3b0 > [147765.739596] [] ? rescuer_thread+0x330/0x330 > [147765.739596] [] ? kthread+0xc1/0xe0 > [147765.739597] [] ? kthread_create_on_node+0x180/0x180 > [147765.739599] [] ? ret_from_fork+0x7c/0xb0 > [147765.739600] [] ? kthread_create_on_node+0x180/0x180 > [147765.739600] ---[ end trace f16ec95e5a50ba64 ]--- > -- 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/fb-helper: Fix hpd vs. initial config races
On Thu, Apr 17, 2014 at 10:38:17AM +0200, Thierry Reding wrote: > On Wed, Apr 16, 2014 at 04:45:21PM +0200, Daniel Vetter wrote: > > Some drivers need to be able to have a perfect race-free fbcon setup. > > Current drivers only enable hotplug processing after the call to > > drm_fb_helper_initial_config which leaves a tiny but important race. > > > > This race is especially noticable on embedded platforms where the > > driver itself enables the voltage for the hdmi output, since only then > > will monitors (after a bit of delay, as usual) respond by asserting > > the hpd pin. > > > > Most of the infrastructure is already there with the split-out > > drm_fb_helper_init. And drm_fb_helper_initial_config already has all > > the required locking to handle concurrent hpd events since > > > > commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f > > Author: Daniel Vetter > > Date: Thu Mar 20 14:26:35 2014 +0100 > > > > drm/fb-helper: improve drm_fb_helper_initial_config locking > > > > The only missing bit is making drm_fb_helper_hotplug_event save > > against concurrent calls of drm_fb_helper_initial_config. The only > > unprotected bit is the check for fb_helper->fb. > > > > With that drivers can first initialize the fb helper, then enabel > > hotplug processing and then set up the initial config all in a > > completely race-free manner. Update kerneldoc and convert i915 as a > > proof of concept. > > > > Feature requested by Thierry since his tegra driver atm reliably boots > > slowly enough to misses the hotplug event for an external hdmi screen, > > but also reliably boots to quickly for the hpd pin to be asserted when > > the fb helper calls into the hdmi ->detect function. > > > > Cc: Thierry Reding > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_fb_helper.c | 11 +-- > > drivers/gpu/drm/i915/i915_dma.c | 3 --- > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > drivers/gpu/drm/i915/i915_irq.c | 4 > > 5 files changed, 5 insertions(+), 16 deletions(-) > > The FB helper parts: > > Tested-by: Thierry Reding > > But I have one comment below... > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c > [...] > > - * Note that the driver must ensure that this is only called _after_ the > > fb has > > - * been fully set up, i.e. after the call to drm_fb_helper_initial_config. > > + * Note that drivers may call this even before calling > > + * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This > > allows > > I don't think the requirement is that strict. To elaborate: on Tegra we > cannot call drm_fb_helper_init() because the number of CRTCs and > connectors isn't known this early. We determine that dynamically after > all sub-devices have been initialized. So instead of calling > drm_fb_helper_init() before drm_kms_helper_poll_init(), I did something > more minimal (see attached patch). It's kind of difficult to tell > because of the context, but tegra_drm_fb_prepare() sets up the mode > config and functions and allocate memory for the FB helper and sets the > FB helper functions. > > This may not be enough for all drivers, but on Tegra the implementation > of .output_poll_changed() simply calls drm_fb_helper_hotplug_event(), > which will work fine with just that rudimentary initialization. Hm yeah I think this should be sufficient, too. It would be good to extract this minimal initialization into a new drm_fb_helper_prepare function and update the kerneldoc a bit more. Maybe as a patch on top of mine? Then we could merge this all as an early tegra-next pull to Dave. -Daniel > > Thierry > From ea394150524c8b54ee4131ad830bf5beb6b1056e Mon Sep 17 00:00:00 2001 > From: Thierry Reding > Date: Thu, 17 Apr 2014 10:02:17 +0200 > Subject: [PATCH] drm/tegra: Implement race-free hotplug detection > > A race condition currently exists on Tegra, where it can happen that a > monitor attached via HDMI isn't detected during the initial FB helper > setup, but the hotplug event happens too early to be processed by the > poll helpers because they haven't been initialized yet. This happens > because on some boards the HDMI driver can control the regulator that > supplies the +5V pin on the HDMI connector. Therefore depending on the > timing between the initialization of the HDMI driver and the rest of > DRM, it's possible that the monitor returns the hotplug signal right > within the window where we would miss it. > > Unfortunately, drm_kms_helper_poll_init() will wreak havoc when called > before at least some parts of the FB helpers have been set up. > > This commit fixes this by splitting out the minimum of initialization > required to make drm_kms_helper_poll_init() work into a separate > function that can be called early. It is then safe to move all of the > poll helper initialization to an earlier point in time (before the > HDMI output driver has a chance to enable the +
[Intel-gfx] Fwd: Eyestrain problems with new Intel drivers
On Thu, Apr 01, 2014 Felix Miata wrote: > On 2014-03-31 10:05 (GMT+0200) Janus composed: >> I have the same problem as the one described by Michael bellow: the >> Intel Graphic card produces eye strain and headache after some minutes >> of using it. I found a lot of people complaining on the same problem, >> but no solution. I tried lowering the resolution, increasing the PWM >> frequency, but without luck. I also installed an old version of intel >> drivers (2.20.12), because the subject of this email was "with **new** >> Intel drivers", but it didn't help. >> >> I have a new HP Folio 9470m, with the Intel HD 4000 graphic card, and >> I cannot use it. Even when connecting an external monitor, I have the >> same problem. When I connect my old laptop, with an nvidia card, to >> the same monitor, the eyestrain disapear, so it must be the Intel >> card, or its driver. >> >> I will be glad to provide any information you consider relevant. I can >> do whatever test you want, but please, do not ignore this message. I >> am not the only one to have this problem: >> https://www.google.com/search?q=Eye+strain+Intel+graphic >> >> I am on (Arch) Linux, but I saw some reports on Windows too. > > What is output from 'lspci | grep VGA'? $ lspci -v -s 00:02.0 00:02.0 VGA compatible controller: Intel Corporation 3rd Gen Core processor Graphics Controller (rev 09) (prog-if 00 [VGA controller]) Subsystem: Hewlett-Packard Company Device 18df Flags: bus master, fast devsel, latency 0, IRQ 49 Memory at d000 (64-bit, non-prefetchable) [size=4M] Memory at c000 (64-bit, prefetchable) [size=256M] I/O ports at 2000 [size=64] Expansion ROM at [disabled] Capabilities: Kernel driver in use: i915 Kernel modules: i915 > Exactly what model is your external display? It is an HP ZR2440w connected through the DisplayPort output of the Docking Station (HP UltraSlim Docking Station). $ xrandr --verbose [...] DP1 connected primary 1920x1200+0+0 (0xf7) normal (normal left inverted right x axis y axis) 518mm x 324mm Identifier: 0x46 Timestamp: 25911732 Subpixel: unknown Gamma: 1.0:1.0:1.0 Brightness: 1.0 Clones: CRTC: 0 CRTCs: 1 0 2 Transform: 1.00 0.00 0.00 0.00 1.00 0.00 0.00 0.00 1.00 filter: EDID: 000022f05429 22160104a534207823fc81a4554d9d25 125054210800d1c081c0814081809500 a940b3000101283c80a070b023403020 36000644211a00fd00183c18 5011000a20202020202000fc0048 50205a5232343430770a202000ff 00434e34323334313150370a20200159 020319c14c901f051404130302070612 01230907078301023a801871382d 40582c45000644211e023a80d072 382d40102c45800644211e011d00 7251d01e206e2855000644211e01 1d00bc52d01e20b8285540064421 1e8c0ad08a20e02d10103e9600064421 187b Broadcast RGB: Automatic supported: Automatic, Full, Limited 16:235 audio: auto supported: force-dvi, off, auto, on 1920x1200 (0xf7) 154.000MHz +HSync -VSync *current +preferred h: width 1920 start 1968 end 2000 total 2080 skew0 clock 74.04KHz v: height 1200 start 1203 end 1209 total 1235 clock 59.95Hz [...] > Does disabling compositing help? No, it doesn't. > Is it better if you boot a Live Linux media from a year or two or > three ago, e.g. Knoppix 7.0.5 or openSUSE 12.3 or Mageia 2 or Fedora > 18? I tested with openSUSE 12.3, but I had the same problem. Thank you for your suggestions. Any other idea? On Mon, Jan 9, 2014 at 3:30 PM, Michael Vanier wrote: > http://lists.freedesktop.org/archives/intel-gfx/2014-January/038104.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Warning on resume
I got this on my desktop (Haswell) box when resuming from suspend with Debian testing kernel (3.13). [147765.739493] [ cut here ] [147765.739501] WARNING: CPU: 1 PID: 29426 at /build/linux-oxWk_8/linux-3.13.7/drivers/gpu/drm/i915/intel_ddi.c:763 intel_ddi_pll_mode_set+0x218/0x8e0 [i915]() [147765.739502] WRPLL already enabled [147765.739513] Modules linked in: nls_utf8 nls_cp437 vfat fat ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT xt_CHECKSUM iptable_mangle xt_tcpudp tun bridge stp llc usb_storage ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables bnep rfcomm bluetooth rfkill cpufreq_conservative cpufreq_stats cpufreq_userspace cpufreq_powersave binfmt_misc sch_fq_codel fuse loop parport_pc ppdev lp parport snd_hda_codec_hdmi snd_hda_codec_realtek x86_pkg_temp_thermal intel_powerclamp iTCO_wdt coretemp iTCO_vendor_support kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel evdev aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper snd_seq_midi snd_hda_intel snd_seq_midi_event cryptd snd_hda_codec psmouse snd_rawmidi serio_raw snd_hwdep snd_pcm_oss snd_mixer_oss lpc_ich i915 mfd_core snd_seq snd_pcm snd_seq_device snd_page_alloc i2c_i801 snd_timer drm_kms_helper snd drm mei _me video mei soundcore processor button ext4 crc16 mbcache jbd2 btrfs xor raid6_pq crc32c libcrc32c dm_mod sg sd_mod sr_mod crc_t10dif cdrom crct10dif_common hid_generic hid_belkin usbhid hid ahci libahci firewire_ohci libata scsi_mod firewire_core crc_itu_t igb ehci_pci xhci_hcd i2c_algo_bit ehci_hcd e1000e i2c_core fan usbcore thermal usb_common thermal_sys ixgbe dca ptp pps_core mdio [147765.739526] CPU: 1 PID: 29426 Comm: kworker/u16:29 Tainted: GW 3.13-1-amd64 #1 Debian 3.13.7-1 [147765.739526] Hardware name: Supermicro X10SAE/X10SAE, BIOS 1.1a 01/03/2014 [147765.739528] Workqueue: events_unbound async_run_entry_fn [147765.739530] 0009 814a1327 8802ae387a88 8105ba72 [147765.739531] 88040b23c000 8802ae387ad8 0001 000278d0 [147765.739532] 880409928000 8105bad7 a0541431 88040018 [147765.739532] Call Trace: [147765.739534] [] ? dump_stack+0x41/0x51 [147765.739535] [] ? warn_slowpath_common+0x72/0x90 [147765.739536] [] ? warn_slowpath_fmt+0x47/0x50 [147765.739542] [] ? gen6_read32+0x48/0x110 [i915] [147765.739548] [] ? intel_ddi_pll_mode_set+0x218/0x8e0 [i915] [147765.739554] [] ? ironlake_enable_vblank+0x26/0x80 [i915] [147765.739560] [] ? haswell_crtc_mode_set+0x36/0x480 [i915] [147765.739566] [] ? __intel_set_mode+0x646/0x1500 [i915] [147765.739571] [] ? gen6_read32+0x48/0x110 [i915] [147765.739577] [] ? intel_modeset_setup_hw_state+0xb19/0xc20 [i915] [147765.739581] [] ? __i915_drm_thaw+0x14a/0x200 [i915] [147765.739585] [] ? i915_resume+0x51/0x70 [i915] [147765.739587] [] ? pci_pm_thaw+0x90/0x90 [147765.739588] [] ? dpm_run_callback+0x2e/0x70 [147765.739589] [] ? device_resume+0x8c/0x180 [147765.739591] [] ? async_resume+0x14/0x40 [147765.739592] [] ? async_run_entry_fn+0x2d/0x120 [147765.739593] [] ? process_one_work+0x16d/0x420 [147765.739594] [] ? worker_thread+0x116/0x3b0 [147765.739596] [] ? rescuer_thread+0x330/0x330 [147765.739596] [] ? kthread+0xc1/0xe0 [147765.739597] [] ? kthread_create_on_node+0x180/0x180 [147765.739599] [] ? ret_from_fork+0x7c/0xb0 [147765.739600] [] ? kthread_create_on_node+0x180/0x180 [147765.739600] ---[ end trace f16ec95e5a50ba64 ]--- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Fwd: 3.15-rc1: Endless "Disabling PPGTT because VT-d is on" messages in /var/log/messages
forwarding from my lkml post as probably more appropriate to send this here... thanks, -- Forwarded message -- From: Alessandro Suardi Date: Fri, Apr 18, 2014 at 12:31 AM Subject: 3.15-rc1: Endless "Disabling PPGTT because VT-d is on" messages in /var/log/messages To: "linux-ker...@vger.kernel.org" [hoping I persuaded gmail to get my old text-only email settings back, grrr] Noticed that my root fs was filling up, and found out that /var/log/messages was approaching 2GB of space, most of which were as per $subject. To give an idea of the magnitude, here's a snippet from a saved sample of /var/log/messages: [root@xbox log]# grep "Disabling PP" messages.save.head |head -1 Apr 15 03:44:10 xbox kernel: [3.115126] [drm] Disabling PPGTT because VT-d is on [root@xbox log]# grep "Disabling PP" messages.save.head |tail -1 Apr 15 03:45:50 xbox kernel: [ 130.165219] [drm] Disabling PPGTT because VT-d is on [root@xbox log]# grep -c "Disabling PP" messages.save.head 610 Or from the dmesg ring: [root@xbox log]# dmesg|head [22446.690572] [drm] Disabling PPGTT because VT-d is on [22446.690616] [drm] Disabling PPGTT because VT-d is on [22446.704560] [drm] Disabling PPGTT because VT-d is on [22446.706747] [drm] Disabling PPGTT because VT-d is on [22446.720202] [drm] Disabling PPGTT because VT-d is on [22446.722447] [drm] Disabling PPGTT because VT-d is on [22446.724850] [drm] Disabling PPGTT because VT-d is on [22446.727268] [drm] Disabling PPGTT because VT-d is on [22446.733052] [drm] Disabling PPGTT because VT-d is on [22446.735319] [drm] Disabling PPGTT because VT-d is on Are there patches for this issue, or is a workaround available to prevent a filesystem full condition on my root fs? This is on a Dell Latitude E6420 laptop, Fedora 19 x86_64, onboard Intel video card... yes, I have an old A08 BIOS, but I've never seen this issue while regularly updating my mainline kernel weekly or so... Available for more details if needed, thanks in advance, --alessandro "don't underestimate the things that I will do" (Adele, "Rolling In The Deep") -- --alessandro "don't underestimate the things that I will do" (Adele, "Rolling In The Deep") ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Eyestrain problems with new Intel drivers
On Thu, Apr 01, 2014 Felix Miata wrote: > On 2014-03-31 10:05 (GMT+0200) Janus composed: >> I have the same problem as the one described by Michael bellow: the >> Intel Graphic card produces eye strain and headache after some minutes >> of using it. I found a lot of people complaining on the same problem, >> but no solution. I tried lowering the resolution, increasing the PWM >> frequency, but without luck. I also installed an old version of intel >> drivers (2.20.12), because the subject of this email was "with **new** >> Intel drivers", but it didn't help. >> >> I have a new HP Folio 9470m, with the Intel HD 4000 graphic card, and >> I cannot use it. Even when connecting an external monitor, I have the >> same problem. When I connect my old laptop, with an nvidia card, to >> the same monitor, the eyestrain disapear, so it must be the Intel >> card, or its driver. >> >> I will be glad to provide any information you consider relevant. I can >> do whatever test you want, but please, do not ignore this message. I >> am not the only one to have this problem: >> https://www.google.com/search?q=Eye+strain+Intel+graphic >> >> I am on (Arch) Linux, but I saw some reports on Windows too. > > What is output from 'lspci | grep VGA'? $ lspci -v -s 00:02.0 00:02.0 VGA compatible controller: Intel Corporation 3rd Gen Core processor Graphics Controller (rev 09) (prog-if 00 [VGA controller]) Subsystem: Hewlett-Packard Company Device 18df Flags: bus master, fast devsel, latency 0, IRQ 49 Memory at d000 (64-bit, non-prefetchable) [size=4M] Memory at c000 (64-bit, prefetchable) [size=256M] I/O ports at 2000 [size=64] Expansion ROM at [disabled] Capabilities: Kernel driver in use: i915 Kernel modules: i915 > Exactly what model is your external display? It is an HP. I do not have access to it now (it is in my office, I can give you the details on Monday). > Does disabling compositing help? No, it doesn't. > Is it better if you boot a Live Linux media from a year or two or > three ago, e.g. Knoppix 7.0.5 or openSUSE 12.3 or Mageia 2 or Fedora > 18? I tested with openSUSE 12.3, but I had the same problem. Thank you for your suggestions. Any other idea? On Mon, Jan 9, 2014 at 3:30 PM, Michael Vanier wrote: > http://lists.freedesktop.org/archives/intel-gfx/2014-January/038104.html -- Alejandro Díaz-Caro http://diaz-caro.info ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
