Re: [Intel-gfx] [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) -Summary- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -1 366/366 365/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_gem_fenced_exec_thrash_no-spare-fences-busy-interruptible PASS(1, M37) DMESG_WARN(1, M37) SNB igt_kms_force_connector NRUN(5, M35M22)PASS(1, M35) NRUN(1, M35) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) HSW igt_kms_force_connector NRUN(5, M40M19M20)PASS(1, M40) NRUN(1, M19) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE][RFC] KVMGT - the implementation of Intel GVT-g(full GPU virtualization) for KVM
On 12/05/2014 09:54 PM, Daniel Vetter wrote: Yeah done a quick read-through of just the i915 bits too, same comment. I guess this is just the first RFC and the redesign we've discussed about already with xengt is in progress somewhere? Yes, it's marching on with Xen now. The KVM implementation is currently not even feature complete - we still have PPGTT missing. Thanks, Daniel -- Thanks, Jike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE][RFC] KVMGT - the implementation of Intel GVT-g(full GPU virtualization) for KVM
CC Andy :) On 12/05/2014 09:03 PM, Paolo Bonzini wrote: On 05/12/2014 09:50, Gerd Hoffmann wrote: A few comments on the kernel stuff (brief look so far, also compile-tested only, intel gfx on my test machine is too old). * Noticed the kernel bits don't even compile when configured as module. Everything (vgt, i915, kvm) must be compiled into the kernel. I'll add that the patch is basically impossible to review with all the XenGT bits still in. For example, the x86 emulator seems to be unnecessary for KVMGT, but I am not 100% sure. This is not ready for merge yet, please wait for a while, we'll have Xen/KVM specific code separated. BTW, definitely you are right, the emulator is unnecessary for KVMGT, and ... unnecessary for XenGT :) I would like a clear understanding of why/how Andrew Barnes was able to do i915 passthrough (GVT-d) without hacking the ISA bridge, and why this does not apply to GVT-g. AFAIK, the graphics drivers need to figure out the offset of some MMIO registers, by the IDs of this ISA bridge. It simply won't work without this information. Talked with Andy about the pass-through but I don't have his implementation, CC Andy for his advice :) Paolo Thanks for review. Would you please also have a look at the issues I mentioned in the original email? they are most KVM-related: the SRCU trickiness, domid, and the memslot created in kernel. Thank you! -- Thanks, Jike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE][RFC] KVMGT - the implementation of Intel GVT-g(full GPU virtualization) for KVM
On 12/05/2014 04:50 PM, Gerd Hoffmann wrote: A few comments on the kernel stuff (brief look so far, also compile-tested only, intel gfx on my test machine is too old). * Noticed the kernel bits don't even compile when configured as module. Everything (vgt, i915, kvm) must be compiled into the kernel. Yes, that's planned to be done along with separating hypervisor-related code from vgt. * Design approach still seems to be i915 on vgt not the other way around. So far yes. Qemu/SeaBIOS bits: I've seen the host bridge changes identity from i440fx to copy-pci-ids-from-host. Guess the reason for this is that seabios uses this device to figure whenever it is running on i440fx or q35. Correct? I did some trick in seabios/qemu. The purpose is to make qemu: - provide IDs of an old host bridge to SeaBIOS - provide IDs of new host bridge(the physical ones) to guest OS So I made seabios to tell qemu that POST is done before jumping to guest OS context. This may be the simplest method to make things work, but yes, q35 emulation of qemu may have this unnecessary, see below. What are the exact requirements for the device? Must it match the host exactly, to not confuse the guest intel graphics driver? Or would something more recent -- such as the q35 emulation qemu has -- be good enough to make things work (assuming we add support for the graphic-related pci config space registers there)? I don't know that is exactly needed, we also need to have Windows driver considered. However, I'm quite confident that, if things gonna work for IGD passthrough, it gonna work for GVT-g. The patch also adds a dummy isa bridge at 0x1f. Simliar question here: What exactly is needed here? Would things work if we simply use the q35 lpc device here? Ditto. more to come after I've read the paper linked above ... Thanks for review :) cheers, Gerd -- Thanks, Jike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] MSR for November / December
Short version: - Khronos face-to-face - BYT performance work Longer version: Yet another Khronos face-to-face meeting. This was a special meeting just for the gl_common working group to hammer out details of XGL (still need a name!) so that we can at least have a chance of having a provisional spec for GDC in March. We made excellent progress on some of the tougher issues, and I think there may actually be a chance of having a usable spec by GDC. There are still some major sticking points. From my POV, the biggest issue is that tile-based renderers (TBR) need some additional information and / or limitations that immediate-mode renderers (IMR) do not. At best an IMR would just drop the data on the floor. At worst an IMR would lose performance due to extra state transitions. On the second day of the meetings there was a very heated 2 hour "debate" about this issue. I think the only thing that came out of it was the obvious conclusion that app developers need to specifically optimize applications for TBRs. While not in meetings or on airplanes, I spent some time looking at where Mesa spends CPU. As soon as I started looking, I couldn't not find problems. I have about 30 patches of potential micro-optimizations across the glUniform paths and the draw-time validation paths. I'd love to send these out, but I'm having quite a bit of difficulty getting meaningful performance data to justify the changes. I have tried several techniques, and I'm not terribly pleased with any of them. The most useful have been: - Use callgrind to get instruction cycle counts. This provides stable results, but it's sloow. It also doesn't account for the shared memory bus or power management interactions. - Use a CPU-limited benchmark and measure its framerate. This provides full-system data, but the results aren't stable. This means you have to do many runs to detect small performance changes. Very small changes may just be undetectable. I wish I had something like shader-db for measuring CPU changes. :( It's worth noting that some of the SynMark2 timings were completely botched. I changed my build scripts to use the same compiler optimization settings as a distro (I picked Fedora) so that I could get results more representative of what real users would see. In the process, I accidentally left --enable-debug in the configure command. This left assertions enabled in the code, and, yeah, that affects performance. Anyway... a few housekeeping patches have already been sent to mesa-dev, and the rest should go out before the end of the year. Next month: All the travel. LCA, another Khronos meeting, and FOSDEM. I'm also taking a week of vacation in January, and sleeping for most of February. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush.
ON these platforms we don't have hardware tracking working for any case. So we need to fake this on software by forcing psr to exit on every flush. Manual tests indicated this was needed. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_psr.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index dd0e6e0..afb8b8c 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -620,13 +620,11 @@ void intel_psr_flush(struct drm_device *dev, /* * On Valleyview and Cherryview we don't use hardware tracking so -* sprite plane updates or cursor moves don't result in a PSR +* any plane updates or cursor moves don't result in a PSR * invalidating. Which means we need to manually fake this in * software for all flushes, not just when we've seen a preceding * invalidation through frontbuffer rendering. */ - if (!HAS_DDI(dev) && - ((frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)) || -(frontbuffer_bits & INTEL_FRONTBUFFER_CURSOR(pipe + if (!HAS_DDI(dev)) intel_psr_exit(dev); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
Since active function on VLV immediately activate PSR let's give more
time for idleness.
v2: Rebase over intel_psr.c and fix typo.
v3: Revival: Manual tests indicated that this is needed. With a short delay
there is a huge
risk of getting blank screens when planes are being enabled.
Signed-off-by: Rodrigo Vivi
Reviewed-by: Durgadoss R
---
drivers/gpu/drm/i915/intel_psr.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index afb8b8c..a6028b6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
+ /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state
+* However on VLV we go to PSR active state with psr work. So let's
+* wait more time. The main reason is to give more time when primary
+* plane is getting enabled avoiding blank screens.
+*/
+ int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
mutex_lock(&dev_priv->psr.lock);
if (!dev_priv->psr.enabled) {
@@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev,
if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
schedule_delayed_work(&dev_priv->psr.work,
- msecs_to_jiffies(100));
+ msecs_to_jiffies(delay));
mutex_unlock(&dev_priv->psr.lock);
}
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] tests/kms_psr_sink_crc: Make blt visible to human eyes
This will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index 98b60cf..ad440a8 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -123,7 +123,7 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color) COLOR_BLIT_COPY_BATCH_START(0); OUT_BATCH((1 << 24) | (0xf0 << 16) | 0); OUT_BATCH(0); - OUT_BATCH(1 << 16 | 4); + OUT_BATCH(0xfff << 16 | 0xfff); OUT_RELOC(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); OUT_BATCH(color); ADVANCE_BATCH(); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] tests/kms_psr_sink_crc: Make plane_move visible to human eyes
this will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index 865ac70..155c25f 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -377,7 +377,7 @@ static void test_crc(data_t *data) break; case PLANE_MOVE: /* Only in use when testing Sprite and Cursor */ - igt_plane_set_position(test_plane, 1, 1); + igt_plane_set_position(test_plane, 500, 500); igt_display_commit(&data->display); break; case PLANE_ONOFF: -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] tests/kms_psr_sink_crc: Make render visible to human eyes
This will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index ad440a8..892bba6 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -139,7 +139,7 @@ static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo) buf->bo = bo; buf->stride = 4096; buf->tiling = I915_TILING_X; - buf->size = 4096; + buf->size = 4; } static void fill_render(data_t *data, uint32_t handle, unsigned char color) @@ -167,7 +167,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color) igt_assert(batch); rendercopy(batch, NULL, - &src_buf, 0, 0, 1, 1, + &src_buf, 0, 0, 0xff, 0xff, &dst_buf, 0, 0); intel_batchbuffer_free(batch); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] tests/kms_psr_sink_crc: Make mmaps visible to human eyes
this will allow manual tests when crc isn't available.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 892bba6..865ac70 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -83,6 +83,7 @@ typedef struct {
drm_intel_bufmgr *bufmgr;
struct igt_fb fb_green, fb_white;
igt_plane_t *primary, *sprite, *cursor;
+ int mod_size;
} data_t;
static void create_cursor_fb(data_t *data)
@@ -335,19 +336,21 @@ static void test_crc(data_t *data)
igt_assert(is_green(crc));
break;
case MMAP_GTT:
- ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+ ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+ PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0xcc, data->mod_size);
+ munmap(ptr, data->mod_size);
break;
case MMAP_GTT_WAITING:
- ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+ ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+ PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
/* Printing white on white so the screen shouldn't change */
- memset(ptr, 0xff, 4);
+ memset(ptr, 0xff, data->mod_size);
get_sink_crc(data, crc);
igt_assert(strcmp(ref_crc, crc) == 0);
@@ -355,15 +358,15 @@ static void test_crc(data_t *data)
sleep(10);
/* Now lets print black to change the screen */
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0, data->mod_size);
+ munmap(ptr, data->mod_size);
break;
case MMAP_CPU:
- ptr = gem_mmap__cpu(data->drm_fd, handle, 0, 4096, PROT_WRITE);
+ ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0, data->mod_size);
+ munmap(ptr, data->mod_size);
gem_sw_finish(data->drm_fd, handle);
break;
case BLT:
@@ -431,6 +434,9 @@ static void run_test(data_t *data)
white_h = mode->hdisplay;
white_v = mode->vdisplay;
+ /* Ignoring pitch and bpp to avoid changing full screen */
+ data->mod_size = white_h * white_v;
+
switch (data->test_plane) {
case SPRITE:
data->sprite = igt_output_get_plane(output,
@@ -454,6 +460,9 @@ static void run_test(data_t *data)
igt_plane_set_fb(data->cursor, NULL);
create_cursor_fb(data);
igt_plane_set_position(data->cursor, 0, 0);
+
+ /* Cursor is 64 x 64, ignoring pitch and bbp again */
+ data->mod_size = 64 * 64;
break;
}
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] tests/kms_psr_sink_crc: Add manual mode.
Sink CRC is the most reliable way to test PSR. However in some platforms
apparently auto generated packages force panel to keep calculating CRC
invalidating
our current sink crc check over debugfs.
So, this manual test help us to find possible gaps on this platforms where we
cannot
trust on sink crc checks.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 74
1 file changed, 68 insertions(+), 6 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 155c25f..3853031 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include "ioctl_wrappers.h"
#include "drmtest.h"
@@ -37,6 +38,7 @@
#include "igt_aux.h"
bool running_with_psr_disabled;
+bool manual;
#define CRC_BLACK ""
@@ -244,6 +246,9 @@ static void get_sink_crc(data_t *data, char *crc) {
int ret;
FILE *file;
+ if (manual)
+ return;
+
file = igt_debugfs_fopen("i915_sink_crc_eDP1", "r");
igt_require(file);
@@ -271,6 +276,9 @@ static bool is_green(char *crc)
unsigned int rh, gh, bh, mask;
int ret;
+ if (manual)
+ return false;
+
sscanf(color_mask, "%4x", &mask);
memcpy(rs, &crc[0], 4);
@@ -293,6 +301,31 @@ static bool is_green(char *crc)
(bh & mask) == 0);
}
+static void assert(bool condition, const char *debug_msg)
+{
+ if (manual) {
+ struct termios oldt, newt;
+ char c;
+
+ igt_info("%s? [Y/n]: ", debug_msg);
+
+ tcgetattr(STDIN_FILENO, &oldt);
+ newt = oldt;
+ newt.c_lflag &= ~(ICANON);
+ tcsetattr(STDIN_FILENO, TCSANOW, &newt);
+ c = getchar();
+ if (c != '\n')
+ igt_info("\n");
+ tcsetattr(STDIN_FILENO, TCSANOW, &oldt);
+
+ if (c == 'n' || c == 'N')
+ igt_fail(-1);
+ } else {
+ igt_debug("%s\n", debug_msg);
+ igt_assert(condition);
+ }
+}
+
static void test_crc(data_t *data)
{
uint32_t handle = data->fb_white.gem_handle;
@@ -300,18 +333,19 @@ static void test_crc(data_t *data)
void *ptr;
char ref_crc[12];
char crc[12];
+ char manual_debug[50] = "";
igt_plane_set_fb(data->primary, &data->fb_green);
igt_display_commit(&data->display);
/* Confirm that screen became Green */
get_sink_crc(data, ref_crc);
- igt_assert(is_green(ref_crc));
+ assert(is_green(ref_crc), "screen GREEN");
/* Confirm screen stays Green after PSR got active */
igt_assert(wait_psr_entry(data, 10));
get_sink_crc(data, ref_crc);
- igt_assert(is_green(ref_crc));
+ assert(is_green(ref_crc), "screen GREEN");
/* Setting a secondary fb/plane */
switch (data->test_plane) {
@@ -325,7 +359,10 @@ static void test_crc(data_t *data)
/* Confirm it is not Green anymore */
igt_assert(wait_psr_entry(data, 10));
get_sink_crc(data, ref_crc);
- igt_assert(!is_green(ref_crc));
+ if (data->test_plane == PRIMARY)
+ assert(!is_green(ref_crc), "screen WHITE");
+ else
+ assert(!is_green(ref_crc), "GREEN background with WHITE box");
switch (data->op) {
case PAGE_FLIP:
@@ -333,7 +370,8 @@ static void test_crc(data_t *data)
igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
data->fb_green.fb_id, 0, NULL) == 0);
get_sink_crc(data, crc);
- igt_assert(is_green(crc));
+ assert(is_green(crc), "screen GREEN");
+ strcpy(manual_debug, "still GREEN");
break;
case MMAP_GTT:
ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -342,6 +380,8 @@ static void test_crc(data_t *data)
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
memset(ptr, 0xcc, data->mod_size);
munmap(ptr, data->mod_size);
+ strcpy(manual_debug,
+ "BLACK or TRANSPARENT mark on top of plane in test");
break;
case MMAP_GTT_WAITING:
ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -352,7 +392,11 @@ static void test_crc(data_t *data)
/* Printing white on white so the screen shouldn't change */
memset(ptr, 0xff, data->mod_size);
get_sink_crc(data, crc);
- igt_assert(strcmp(ref_crc, crc) == 0);
+ if (data->test_plane == PRIMARY)
+ assert(strcmp(ref_crc, crc) == 0, "screen WHITE");
+ else
+ assert(strcmp(ref_crc, crc) == 0,
+
Re: [Intel-gfx] [PATCH] drm/i915/chv: Enable HDMI Clock recovery for Pipe C
On 12/05/2014 05:23 AM, Imre Deak wrote: On Fri, 2014-12-05 at 13:57 +0200, Jani Nikula wrote: On Fri, 05 Dec 2014, Imre Deak wrote: On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote: On Fri, 05 Dec 2014, Clint Taylor wrote: On 12/04/2014 11:08 AM, Clint Taylor wrote: On 12/04/2014 12:41 AM, Jani Nikula wrote: On Wed, 03 Dec 2014, Clint Taylor wrote: On 12/03/2014 01:01 PM, Ville Syrjälä wrote: On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.tay...@intel.com wrote: From: Clint Taylor Added PIPE C register support for CHV audio programming. nak. The offset between the pipes looks constant so it should work just fine with _PIPE(). Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after ChromeOS boot. "after boot" is a long time! ;) Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets called? With drm.debug=14 you should see "Disable audio codec on port %c, pipe %c\n". BR, Jani. I've actually instrumented the driver with more logging and at 22.43 in the dmesg log the last call to audio_codec_enable() is called and is setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for 1080p60. Sometime after 22.436 and before the ChromeOS login screen appears the register is being cleared to 0x0 without knowledge/action of the i915 driver. Mode change and power state change seem to restore the programming correctly. Most likely candidate right now is an ordering issue with snd_hda during boot. Ok, removing snd_hda drivers from the boot process did not change the result. Any ideas about what could reset the HD_AUDIO registers to their default? Maybe it needs some power domain/well we fail to get? Similar to what DDI based platforms do before and after calls to intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c. Not really my area, maybe Imre and Ville know better? The audio power domain->power well mapping is mapped correctly in the i915 driver (HDA belongs to the display aka pipe-A power well there). But AFAICS the intel_hda driver doesn't ask for this power domain on VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda device capability flags. CC'ing Takashi for this. Imre, i915_request_power_well() only works for hsw/bdw. Hm, right I should have checked that first. And it could be also the reason why AZX_DCAPS_I915_POWERWELL was left out for everything but HSW/BDW. I will send a patch to fix that in i915. Thanks, Imre I have applied Imre's request_power_well patch, and tried setting intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO) directly when the audio codec is enabled and the register is still not accepting pixel clock programming updates during boot. I have found that the register will not accept the value until after snd_hda_intel initializes the PCI device. Unfortunately time wise this occurs after the last attempt made by the i915 driver. I will continue to investigate next week. [ 25.993784] *** CAT display power get [ 25.998016] *** CAT enable aud_config_c read = 0x [ 31.825282] snd_hda_intel :00:1b.0: irq 403 for MSI/MSI-X -Clint BR, Jani. Still, I'm not sure why the register gets zeroed, if there is no power well off->on transition. You could first check if this is really true, via the debug print intel_display_power_get() and intel_display_power_put() emits. Another idea would be to trace writes to this register through I915_WRITE. --Imre BR, Jani. -clint [8.607022] [drm:intel_audio_codec_enable] ELD on [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] [8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port D, pipe C, 8 bytes ELD [8.607044] *** CAT enable aud_config reg 0x001E2200 = 0x0009 [ 14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port D, pipe C [ 14.741055] *** CAT disable aud_config reg 0x001E2200 = 0x1000 [ 15.773618] [drm:intel_audio_codec_enable] ELD on [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] [ 15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port D, pipe C, 8 bytes ELD [ 15.779315] *** CAT enable aud_config reg 0x001E2200 = 0x0009 [ 19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port D, pipe C [ 19.936778] *** CAT disable aud_config reg 0x001E2200 = 0x1000 [ 20.292920] [drm:intel_audio_codec_enable] ELD on [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] [ 20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port D, pipe C, 8 bytes ELD [ 20.298615] *** CAT enable aud_config reg 0x001E2200 = 0x0009 [ 20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port D, pipe C [ 20.389121] *** CAT disable aud_config reg 0x001E2200 = 0x1000 [ 22.430931] [drm:intel_audio_codec_enable] ELD on [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] [ 22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port D, pipe C, 8 bytes ELD [ 22.436627] *** CAT enable aud_config
Re: [Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) -Summary- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -4 366/366 362/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -Detailed- Platform Testdrm-intel-nightly Series Applied ILK igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible DMESG_WARN(1, M26)PASS(4, M37M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_flip-vs-panning PASS(2, M37M26) DMESG_WARN(1, M26) ILK igt_kms_flip_plain-flip-ts-check-interruptible DMESG_WARN(1, M26)PASS(1, M37) DMESG_WARN(1, M26) *ILK igt_kms_flip_rcs-flip-vs-modeset PASS(3, M37M26) DMESG_WARN(1, M26) SNB igt_kms_force_connector NRUN(4, M35M22)PASS(1, M35) NRUN(1, M35) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(14, M4M34M21) PASS(1, M34) HSW igt_kms_force_connector NRUN(4, M40M19M20)PASS(1, M40) NRUN(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: vlv: clamp minimum RPS frequency to what Punit allows
On Thu, Dec 04, 2014 at 06:39:35PM +0200, Imre Deak wrote:
> As described in the code comment, I couldn't set the minimum RPS
> frequency on my BYT-M B0 to the minimum allowed as reported by Punit.
> Fix this by clamping the minimum value to the first one that was
> accepted on my machine. This fixes at least the pm_rpm basic-api and
> min-max-config subtests.
>
> Testcase: igt/pm_rps
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_pm.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 45c786f..7a1112f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5039,7 +5039,17 @@ static int valleyview_rps_rpe_freq(struct
> drm_i915_private *dev_priv)
>
> static int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> {
> - return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> + u32 val;
> +
> + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> + /*
> + * According to the BYT Punit GPU turbo HAS 1.1.6.3 the minimum value
> + * for the minimum frequency in GPLL mode is 0xc1. Contrary to this on
> + * a BYT-M B0 the above register contains 0xbf. Moreover when setting
> + * a frequency Punit will not allow values below 0xc0. Clamp it 0xc0
> + * to make sure it matches what Punit accepts.
> + */
> + return max_t(u32, val, 0xc0);
Matches what I see on this ffrd as well. But this too has czclk==266.
Would be interesting to see what happens with other czclks. Anyone
have a byt w/ 1333 memory?
> }
>
> /* Check that the pctx buffer wasn't move under us. */
> --
> 1.8.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: vlv: clamp minimum RPS frequency to what Punit allows
On Fri, 2014-12-05 at 23:16 +0200, Imre Deak wrote:
> On Fri, 2014-12-05 at 21:58 +0100, Daniel Vetter wrote:
> > On Thu, Dec 04, 2014 at 06:39:35PM +0200, Imre Deak wrote:
> > > As described in the code comment, I couldn't set the minimum RPS
> > > frequency on my BYT-M B0 to the minimum allowed as reported by Punit.
> > > Fix this by clamping the minimum value to the first one that was
> > > accepted on my machine. This fixes at least the pm_rpm basic-api and
> > > min-max-config subtests.
> > >
> > > Testcase: igt/pm_rps
> > > Signed-off-by: Imre Deak
> >
> > Isn't there a bugzilla for this, too?
>
> I didn't check this:/ There are some that may be fixed, or at least
> partially fixed. I just list all pm_rps bugs here, they are all worth
> rechecking imo, although the non-VLV/CHV ones are less likely to be
> related:
>
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=80704
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=86654
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=86363
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=84896
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=77869
Sorry, I meant the above for the 3 igt patches actually. For this
particular one I haven't found any ticket.
>
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_pm.c | 12 +++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 45c786f..7a1112f 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -5039,7 +5039,17 @@ static int valleyview_rps_rpe_freq(struct
> > > drm_i915_private *dev_priv)
> > >
> > > static int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> > > {
> > > - return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> > > + u32 val;
> > > +
> > > + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> > > + /*
> > > + * According to the BYT Punit GPU turbo HAS 1.1.6.3 the minimum value
> > > + * for the minimum frequency in GPLL mode is 0xc1. Contrary to this on
> > > + * a BYT-M B0 the above register contains 0xbf. Moreover when setting
> > > + * a frequency Punit will not allow values below 0xc0. Clamp it 0xc0
> > > + * to make sure it matches what Punit accepts.
> > > + */
> > > + return max_t(u32, val, 0xc0);
> > > }
> > >
> > > /* Check that the pctx buffer wasn't move under us. */
> > > --
> > > 1.8.4
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: vlv: clamp minimum RPS frequency to what Punit allows
On Fri, 2014-12-05 at 21:58 +0100, Daniel Vetter wrote:
> On Thu, Dec 04, 2014 at 06:39:35PM +0200, Imre Deak wrote:
> > As described in the code comment, I couldn't set the minimum RPS
> > frequency on my BYT-M B0 to the minimum allowed as reported by Punit.
> > Fix this by clamping the minimum value to the first one that was
> > accepted on my machine. This fixes at least the pm_rpm basic-api and
> > min-max-config subtests.
> >
> > Testcase: igt/pm_rps
> > Signed-off-by: Imre Deak
>
> Isn't there a bugzilla for this, too?
I didn't check this:/ There are some that may be fixed, or at least
partially fixed. I just list all pm_rps bugs here, they are all worth
rechecking imo, although the non-VLV/CHV ones are less likely to be
related:
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=80704
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=86654
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=86363
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=84896
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=77869
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 45c786f..7a1112f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5039,7 +5039,17 @@ static int valleyview_rps_rpe_freq(struct
> > drm_i915_private *dev_priv)
> >
> > static int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> > {
> > - return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> > + u32 val;
> > +
> > + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> > + /*
> > +* According to the BYT Punit GPU turbo HAS 1.1.6.3 the minimum value
> > +* for the minimum frequency in GPLL mode is 0xc1. Contrary to this on
> > +* a BYT-M B0 the above register contains 0xbf. Moreover when setting
> > +* a frequency Punit will not allow values below 0xc0. Clamp it 0xc0
> > +* to make sure it matches what Punit accepts.
> > +*/
> > + return max_t(u32, val, 0xc0);
> > }
> >
> > /* Check that the pctx buffer wasn't move under us. */
> > --
> > 1.8.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/4] tests/kms_flip: Target the back buffer with the dummy load
On Fri, Dec 05, 2014 at 05:13:17PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Aim the dummy load to the current back buffer instead if the front > buffer. Assuming the idea is to get the next flip to be stuck behind > the dummy load? Yeah that makes more sense. Ack on the entire series. I still think it'd be useful to extract a solid dummy load tuner into lib, but that's a bit more work really. I'll do an igt jira for it so we don't forget. -Daniel > > Signed-off-by: Ville Syrjälä > --- > tests/kms_flip.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > index 5d5302d..f847a86 100644 > --- a/tests/kms_flip.c > +++ b/tests/kms_flip.c > @@ -878,15 +878,15 @@ static unsigned int run_test_step(struct test_output *o) > if (o->flags & TEST_DPMS_OFF_OTHERS) > dpms_off_other_outputs(o); > > + if (!(o->flags & TEST_SINGLE_BUFFER)) > + o->current_fb_id = !o->current_fb_id; > + > if (o->flags & TEST_WITH_DUMMY_BCS) > emit_dummy_load__bcs(o, 1); > > if (o->flags & TEST_WITH_DUMMY_RCS) > emit_dummy_load__rcs(o, 1); > > - if (!(o->flags & TEST_SINGLE_BUFFER)) > - o->current_fb_id = !o->current_fb_id; > - > if (o->flags & TEST_FB_RECREATE) > recreate_fb(o); > new_fb_id = o->fb_ids[o->current_fb_id]; > -- > 2.0.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 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests
On Fri, Dec 05, 2014 at 08:59:42PM +, Chris Wilson wrote: > On Fri, Dec 05, 2014 at 09:56:18PM +0100, Daniel Vetter wrote: > > On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote: > > > When changing the sysfs GT min/max frequencies, the kernel won't > > > explicitly change the current frequency, unless it becomes out of bound > > > based on the new min/max values. The test happens to work on non-VLV > > > platforms because on those the kernel resets the current frequency > > > unconditionally (to adjust the RPS interrupt mask as a side-effect) and > > > that will lead to an RPS interrupt setting the minimum frequency. > > > > > > To fix this load the GPU after decreasing the min frequency and before > > > checking the current frequency. This should set the current frequency to > > > the minimum. > > > > > > Signed-off-by: Imre Deak > > > > Isn't this a kernel bug which should be fixed in the kernel? If userspace > > changes the requested freq, the kernel should obey right away. Not > > somewhen later when something happens ... > > Only if busy. If the GPU is idle, we should ignore the softlimits and > program the most efficient idle frequency - as part of i915.powersave. Hm yeah that slight reinterpretation of the limits makes them actually more useful. So ack from me on all 3 patches. -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: Detect page faults during hangcheck
On Fri, Dec 05, 2014 at 02:15:21PM +, Chris Wilson wrote:
> On Sandybridge+, the GPU provides the ERROR register for detecting page
> faults. Hook this up to our hangcheck so that we can dump the error
> state soon after such an event occurs. This would be better inside an
> interrupt handler, but it serves a purpose here as it detects that our
> initial context setup is invalid...
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_irq.c | 5 +
> drivers/gpu/drm/i915/intel_uncore.c | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 7913a72ce30a..eb2149b941e4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2969,6 +2969,11 @@ static void i915_hangcheck_elapsed(unsigned long data)
> if (!i915.enable_hangcheck)
> return;
>
> + if (INTEL_INFO(dev_priv)->gen >= 6 && I915_READ(ERROR_GEN6)) {
> + i915_handle_error(dev, false, "GPU reported a page fault");
Is the full hangcheck state actually useful for debugging these
pagefaults? The gpu doesn't seem to fall over completely, so I guess ACTHD
and friends are somewhere in nirvana.
Enabling the interrupts would definitely be useful though. I think all the
handler code is written already (perhaps a few missing drm_debug lines),
it's just that we don't enable these interrupt sources by default.
-Daniel
> + I915_WRITE(ERROR_GEN6, 0);
> + }
> +
> for_each_ring(ring, dev_priv, i) {
> u64 acthd;
> u32 seqno;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 0655b44651a7..67f2e24c5bc5 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -535,6 +535,8 @@ static void __intel_uncore_early_sanitize(struct
> drm_device *dev,
> if (IS_GEN6(dev) || IS_GEN7(dev))
> __raw_i915_write32(dev_priv, GTFIFODBG,
> __raw_i915_read32(dev_priv, GTFIFODBG));
> + if (INTEL_INFO(dev)->gen >= 6)
> + __raw_i915_write32(dev_priv, ERROR_GEN6, 0);
>
> intel_uncore_forcewake_reset(dev, restore_forcewake);
> }
> --
> 2.1.3
>
> ___
> 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/3] tests/pm_rps: vlv: load gpu for idle min/max tests
On Fri, 2014-12-05 at 21:56 +0100, Daniel Vetter wrote: > On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote: > > When changing the sysfs GT min/max frequencies, the kernel won't > > explicitly change the current frequency, unless it becomes out of bound > > based on the new min/max values. The test happens to work on non-VLV > > platforms because on those the kernel resets the current frequency > > unconditionally (to adjust the RPS interrupt mask as a side-effect) and > > that will lead to an RPS interrupt setting the minimum frequency. > > > > To fix this load the GPU after decreasing the min frequency and before > > checking the current frequency. This should set the current frequency to > > the minimum. > > > > Signed-off-by: Imre Deak > > Isn't this a kernel bug which should be fixed in the kernel? If userspace > changes the requested freq, the kernel should obey right away. Not > somewhen later when something happens ... It isn't the current frequency that's changed but the min/max limits. If by changing those the current frequency gets out of bound the kernel properly changes the current frequency too. > -Daniel ___ 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: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 04:23:56PM +, Chris Wilson wrote:
> On Fri, Dec 05, 2014 at 03:58:40PM +0100, Daniel Vetter wrote:
> > On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> > > On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > > > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > > > Currently we initialise the rings, add the first context switch to the
> > > > > ring and execute our golden state then enable (aliasing or full)
> > > > > ppgtt.
> > > > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > > > MI_LRI, we end up executing the context switch and golden render state
> > > > > with an invalid PD generating page faults. To solve this issue, first
> > > > > do
> > > > > the ppgtt PD setup, then set the default context and write the
> > > > > commands
> > > > > to run the render state into the ring, before we activate the ring.
> > > > > This
> > > > > allows us to be sure that the register state is valid before we begin
> > > > > execution.
> > > > >
> > > > > This was spotted when writing the seqno/request conversion, but only
> > > > > with
> > > > > the ERROR capture did I realise that it was a necessity now.
> > > > >
> > > > > RFC: cleanup the error handling in i915_gem_init_hw.
> > > > >
> > > > > Signed-off-by: Chris Wilson
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > > index c1c11418231b..c13842d3cbc9 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > > > >*/
> > > > > init_unused_rings(dev);
> > > > >
> > > > > - for_each_ring(ring, dev_priv, i) {
> > > > > - ret = ring->init_hw(ring);
> > > > > - if (ret)
> > > > > - return ret;
> > > > > - }
> > > > > -
> > > > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> > > >
> > > > This is going to assume ring->head/tail are already valid?
> > >
> > > We write into the ring obj, not the ring itself, which should be setup
> > > during the various intel_init_engine, i.e. the backing storage is
> > > independent of the actual registers.
> >
> > But there's still intel_ring_advance which calls ->write_tail all over the
> > place. So we drop all these mmio writes into nirvana since we'll reset the
> > ring later on?
>
> intel_ring_advance() doesn't do the register update, it just updates
> ring->tail. And even if it did, whilst the ring is disabled, nobody is
> listening, right?
Oh right, mixed that up. So all well from my pov for this 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 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests
On Fri, Dec 05, 2014 at 09:56:18PM +0100, Daniel Vetter wrote: > On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote: > > When changing the sysfs GT min/max frequencies, the kernel won't > > explicitly change the current frequency, unless it becomes out of bound > > based on the new min/max values. The test happens to work on non-VLV > > platforms because on those the kernel resets the current frequency > > unconditionally (to adjust the RPS interrupt mask as a side-effect) and > > that will lead to an RPS interrupt setting the minimum frequency. > > > > To fix this load the GPU after decreasing the min frequency and before > > checking the current frequency. This should set the current frequency to > > the minimum. > > > > Signed-off-by: Imre Deak > > Isn't this a kernel bug which should be fixed in the kernel? If userspace > changes the requested freq, the kernel should obey right away. Not > somewhen later when something happens ... Only if busy. If the GPU is idle, we should ignore the softlimits and program the most efficient idle frequency - as part of i915.powersave. -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: Make drm_read() more robust against multithreaded races
On Thu, Dec 04, 2014 at 06:19:54PM -0800, shuang...@intel.com wrote: > Tested-By: PRC QA PRTS (Patch Regression Test System Contact: > shuang...@intel.com) > -Summary- > Platform Delta drm-intel-nightly Series Applied > PNV 364/364 364/364 > ILK 366/366 366/366 > SNB 450/450 450/450 > IVB +17 481/498 498/498 Today there have been a lot of prts test results with +17 for ivb. Is something wrong with the baseline results that there's always the same regressions/improvements? In general there's an awful lot of noise in prts results, and off-by-one in comparing results would explain a lot ... -Daniel > BYT 289/289 289/289 > HSW 564/564 564/564 > BDW 417/417 417/417 > -Detailed- > Platform Testdrm-intel-nightly > Series Applied > IVB igt_kms_3d DMESG_WARN(1, M34)PASS(10, M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(10, M4M34M21) PASS(1, > M34) > IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(10, > M4M34M21) PASS(1, M34) > Note: You need to pay more attention to line start with '*' > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: vlv: clamp minimum RPS frequency to what Punit allows
On Thu, Dec 04, 2014 at 06:39:35PM +0200, Imre Deak wrote:
> As described in the code comment, I couldn't set the minimum RPS
> frequency on my BYT-M B0 to the minimum allowed as reported by Punit.
> Fix this by clamping the minimum value to the first one that was
> accepted on my machine. This fixes at least the pm_rpm basic-api and
> min-max-config subtests.
>
> Testcase: igt/pm_rps
> Signed-off-by: Imre Deak
Isn't there a bugzilla for this, too?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_pm.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 45c786f..7a1112f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5039,7 +5039,17 @@ static int valleyview_rps_rpe_freq(struct
> drm_i915_private *dev_priv)
>
> static int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> {
> - return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> + u32 val;
> +
> + val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> + /*
> + * According to the BYT Punit GPU turbo HAS 1.1.6.3 the minimum value
> + * for the minimum frequency in GPLL mode is 0xc1. Contrary to this on
> + * a BYT-M B0 the above register contains 0xbf. Moreover when setting
> + * a frequency Punit will not allow values below 0xc0. Clamp it 0xc0
> + * to make sure it matches what Punit accepts.
> + */
> + return max_t(u32, val, 0xc0);
> }
>
> /* Check that the pctx buffer wasn't move under us. */
> --
> 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 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests
On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote: > When changing the sysfs GT min/max frequencies, the kernel won't > explicitly change the current frequency, unless it becomes out of bound > based on the new min/max values. The test happens to work on non-VLV > platforms because on those the kernel resets the current frequency > unconditionally (to adjust the RPS interrupt mask as a side-effect) and > that will lead to an RPS interrupt setting the minimum frequency. > > To fix this load the GPU after decreasing the min frequency and before > checking the current frequency. This should set the current frequency to > the minimum. > > Signed-off-by: Imre Deak Isn't this a kernel bug which should be fixed in the kernel? If userspace changes the requested freq, the kernel should obey right away. Not somewhen later when something happens ... -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 5/5] drm/i915: Dual link needs Shutdown and Turn on packet for both ports
On Sat, Dec 06, 2014 at 12:40:39AM +0530, Gaurav K Singh wrote: > For dual link MIPI panels, SHUTDOWN packet needs to send to both Ports > A & C during MIPI encoder disabling sequence. Similarly, TURN ON packet > to be sent to both Ports during MIPI encoder enabling sequence. > > v2: Address review comments by Jani > - Used a for loop instead of do-while loop. > > v3: Used for_each_dsi_port macro instead of for loop > > Signed-off-by: Gaurav K Singh > Signed-off-by: Shobhit Kumar > Reviewed-by: Jani Nikula I've merged this one already, so if you have changes please provide them as fixups on top of -nightly. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Additional request structure tracing
On Fri, Dec 05, 2014 at 09:48:03PM +0100, Daniel Vetter wrote: > On Fri, Dec 05, 2014 at 01:49:36PM +, john.c.harri...@intel.com wrote: > > From: John Harrison > > > > Added the request structure's 'uniq' identifier to the trace information. > > Also > > renamed the '_complete' trace event to '_notify' as it actually happens in > > the > > IRQ 'notify_ring()' function. The intention is to add a new '_complete' > > trace > > event which occurs when a request structure is actually marked as complete. > > However, at the moment the completion status is re-tested every time the > > query > > is made so there isn't a completion event as such. > > > > v2: New patch added to series. > > > > v3: Rebased to remove completion caching as that is apparently contentious. > > > > Change-Id: Ic9bcde67d175c6c03b96217cdcb6e4cc4aa45d67 > > For: VIZ-4377 > > Signed-off-by: John Harrison > > Reviewed-by: Thomas Daniel > > Ok I guess you'll hate this by now, but struct fence provides all this > already, too. It doesn't use the uniq id trick you have put instead just > uses the pointer. But there's create/destroy tracepoints too afaik, so I > think we're covered. > > I've merged the first two patches from this series, thanks. Well changed my mind again since it's just an extension of what we have. So patches 3&4 are merged, too. But yeah I think we really sould switch over to the fence stuff for all this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Additional request structure tracing
On Fri, Dec 05, 2014 at 01:49:36PM +, john.c.harri...@intel.com wrote: > From: John Harrison > > Added the request structure's 'uniq' identifier to the trace information. Also > renamed the '_complete' trace event to '_notify' as it actually happens in the > IRQ 'notify_ring()' function. The intention is to add a new '_complete' trace > event which occurs when a request structure is actually marked as complete. > However, at the moment the completion status is re-tested every time the query > is made so there isn't a completion event as such. > > v2: New patch added to series. > > v3: Rebased to remove completion caching as that is apparently contentious. > > Change-Id: Ic9bcde67d175c6c03b96217cdcb6e4cc4aa45d67 > For: VIZ-4377 > Signed-off-by: John Harrison > Reviewed-by: Thomas Daniel Ok I guess you'll hate this by now, but struct fence provides all this already, too. It doesn't use the uniq id trick you have put instead just uses the pointer. But there's create/destroy tracepoints too afaik, so I think we're covered. I've merged the first two patches from this series, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
On Fri, Dec 05, 2014 at 06:02:35PM +, John Harrison wrote: > But yes, the reasoning why it crashes without the zero fill is correct. > Dodgy context pointers that used to be ignored now get processed. Doing the > zero fill keeps it all sane. > > On 05/12/2014 17:54, John Harrison wrote: > >This is already part of the seqno/request patch series and has been right > >from the start. See email 'drm/i915: Zero fill the request structure'. > > > >On 05/12/2014 17:54, Mika Kuoppala wrote: > >>Otherwise we might end up referencing uninitialized fields. > >>This is apparent when we try to cleanup the preallocated request > >>on ring reset, before any request has been submitted to the ring. > >>The request->ctx is foobar and we end up freeing the foobarness. > >> > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 > >>Cc: John Harrison > >>Signed-off-by: Mika Kuoppala I've added this explanation plus the note about which commit introduced the regression to John's patch and merged that one. Thanks for tracking this down. -Daniel > >>--- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>index 79b4ca5..2c6c6f8 100644 > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >>@@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs > >>*ring) > >> if (ring->outstanding_lazy_request) > >> return 0; > >> -request = kmalloc(sizeof(*request), GFP_KERNEL); > >>+request = kzalloc(sizeof(*request), GFP_KERNEL); > >> if (request == NULL) > >> return -ENOMEM; > > > > ___ > 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 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/5/2014 11:18 PM, Siluvery, Arun wrote:
On 05/12/2014 17:36, Jani Nikula wrote:
On Fri, 05 Dec 2014, "Siluvery, Arun"
wrote:
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock.
Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock
will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
From the host perspective a dual link (or dual channel) DSI device is
two independent peripheral devices. On the peripheral side the display
has to combine the input from the two links (which may be two
independent DSI blocks on the peripheral as well) into one contiguous
display. I don't know the details, but I'm guessing pixel overlap just
makes it easier for the peripheral implementation to get it all
together.
Thank you for the details.
I am just wondering how few extra pixels help on the display side
unless they are fixed values which act like some kind of markers to
synchronize between two halves.
regards
Arun
Pixel overlap count are fixed values only and it can have values of 0, 1
or 2. These values will be defined for the particular MIPI DSI panel specs.
These values are programmed in VBT which driver parses and uses it.
With regards,
Gaurav
BR,
Jani.
regards
Arun
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21
+
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31)
#define VLV_PWRDWNUPCTL0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow,
but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not
been updated in the BSpec. Also, it was communicated by the BIOS
team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT2
u16 dual_link:2;
u16 lane_cnt:2;
-u16 rsvd3:12;
+u16 pixel_overlap:3;
+u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct
intel_encoder *encoder)
enum port port;
u32 temp;
+if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+temp = I915_READ(VLV_CHICKEN_3);
+temp &= ~PIXEL_OVERLAP_CNT_MASK |
+intel_dsi->pixel_overlap <<
+PIXEL_OVERLAP_CNT_SHIFT;
+I915_WRITE(VLV_CHICKEN_3, temp);
+}
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE0
+#define DSI_DUAL_LINK_FRONT_BACK1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 1
Re: [Intel-gfx] [PATCH v4 1/4] drm/i915: Fix up seqno -> request merge issues
On Fri, Dec 05, 2014 at 01:49:33PM +, john.c.harri...@intel.com wrote: > From: John Harrison > > The display related patches earlier in this series were edited during merge to > improve the request unreferencing. Specifically, the need for de-referencing > at > interrupt time was removed. However, the resulting code did a 'deref(req) ; > req > = NULL' sequence rather than using the 'req_assign(req, NULL)' wrapper. The > two > are functionally equivalent, but using the wrapper is more consistent with all > the other places where requests are assigned. > > Note that the whole point of the wrapper is that using it everywhere that > request pointers are assigned means that the reference counting is done > automatically and can't be accidentally forgotten about. Plus it allows > simpler > future maintainance if the reference counting mechanisms ever need to change. > > For: VIZ-4377 > Signed-off-by: John Harrison I guess it's fairly clear that i915 hasn't used this patter previously ;-) Thanks for fixing this up. Aside, if you have a slow day: Coccinelle might be able to automatically hunt for refactorings like this for you. And even if not playing with that tool is worth it all on its own. -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 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller
On 12/5/2014 8:08 PM, Daniel Vetter wrote: On Fri, Dec 05, 2014 at 06:20:43PM +0530, Singh, Gaurav K wrote: On 12/4/2014 4:52 PM, Daniel Vetter wrote: On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote: On Thu, 04 Dec 2014, Gaurav K Singh wrote: This patch is in preparation of DSI dual link panels. For dual link panels, few packets needs to be sent to Port A or Port C or both. Based on the portno from MIPI Sequence Block#53, these sequences needs to be sent accordingly. v2: Addressed review comments by Jani - port variables named properly Signed-off-by: Gaurav K Singh Reviewed-by: Jani Nikula Merged the first two patches, thanks. -Daniel Hi Daniel, Thanks. I have addressed Jani's review comments too for the rest of the patches. Well looking through the resend patches your changes look a bit minimal. Which might be the right thing to do, but review is also about communication and sharing expert knowledge. So please follow up to the questions from Jani where you didn't adjust the code. And if there's any follow-up patches from those discussions please submit them. Another part: Your editor doesn't seem to align continuation lines quite how we usually do that. checkpatch --strict will catch that. Two big rules: - function paramater lists should be aligned to the opening ( on the next line. - Continuations of boolean logic checks (e.g. in if checks) same, with the special rule that && or || should be on the previous line as the last thing. - continuation of any other long lines should be indented 1 or two spaces (just to make sure it sticks out without looking jarring). Especially the first two help readability a lot of you're used to them, so please do a follow-up patch to rectify this in intel_dsi*.c. Thanks, Daniel Done. Fixed the style related issues and comments also added. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't complain about stolen conflicts on gen3
On Fri, Dec 05, 2014 at 10:30:47AM -0800, Jesse Barnes wrote:
> On Fri, 11 Apr 2014 15:55:17 +0200
> Daniel Vetter wrote:
>
> > Apparently stuff works that way on those machines.
> >
> > I agree with Chris' concern that this is a bit risky but imo worth a
> > shot in -next just for fun. Afaics all these machines have the pci
> > resources allocated like that by the BIOS, so I suspect that it's all
> > ok.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76983
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71031
> > Tested-by: lu hua
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c index
> > 62ef55ba061c..99d147af173a 100644 ---
> > a/drivers/gpu/drm/i915/i915_gem_stolen.c +++
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -93,7 +93,11 @@ static
> > unsigned long i915_stolen_to_physical(struct drm_device *dev) r =
> > devm_request_mem_region(dev->dev, base + 1, dev_priv->gtt.stolen_size
> > - 1, "Graphics Stolen Memory");
> > - if (r == NULL) {
> > + /*
> > +* GEN3 firmware likes to smash pci bridges into the
> > stolen
> > +* range. Apparently this works.
> > +*/
> > + if (r == NULL && !IS_GEN3(dev)) {
> > DRM_ERROR("conflict detected with stolen
> > region: [0x%08x - 0x%08x]\n", base, base +
> > (uint32_t)dev_priv->gtt.stolen_size); base = 0;
>
>
> Yeah just to allay fears: the decode priority on the GMCH is fixed and
> specific. The stolen range is demarcated by some regs which the GMCH
> decodes before it tries going out into PCI space. So it's safe to see
> the stolen range under the bus0 window (probably even under some device
> window down the range) but does make things messier for us.
>
> Reviewed-by: Jesse Barnes
>
> Looks like the reporter gave a t-b too.
The other t-b from the other bugzilla is missing though:
Tested-by: Paul Menzel
Cc: sta...@vger.kernel.org
This regression goes back to
commit eaba1b8f3379b5d100bd146b9a41d28348bdfd09
Author: Chris Wilson
Date: Thu Jul 4 12:28:35 2013 +0100
drm/i915: Verify that our stolen memory doesn't conflict
Jani, can you please pick this up for 3.19?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: Update the DSI enable path to support dual
We need to program both port registers during dual link enable path.
v2: Address review comments by Jani
- Used a for loop instead of do-while loop.
v3: Used for_each_dsi_port macro instead of for loop
v4: Renamed mode_hactive variable to mode_hdisplay
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
Reviewed-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dsi.c | 244 +-
1 file changed, 134 insertions(+), 110 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 8054928..84e0231 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -156,8 +156,8 @@ static void intel_dsi_port_disable(struct intel_encoder
*encoder)
static void intel_dsi_device_ready(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
- enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ enum port port;
u32 val;
DRM_DEBUG_KMS("\n");
@@ -171,18 +171,21 @@ static void intel_dsi_device_ready(struct intel_encoder
*encoder)
/* bandgap reset is needed after everytime we do power gate */
band_gap_reset(dev_priv);
- I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
- usleep_range(2500, 3000);
+ for_each_dsi_port(port, intel_dsi->ports) {
- val = I915_READ(MIPI_PORT_CTRL(port));
- I915_WRITE(MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
- usleep_range(1000, 1500);
+ I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
+ usleep_range(2500, 3000);
- I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
- usleep_range(2500, 3000);
+ val = I915_READ(MIPI_PORT_CTRL(port));
+ I915_WRITE(MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
+ usleep_range(1000, 1500);
- I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
- usleep_range(2500, 3000);
+ I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
+ usleep_range(2500, 3000);
+
+ I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
+ usleep_range(2500, 3000);
+ }
}
static void intel_dsi_enable(struct intel_encoder *encoder)
@@ -547,32 +550,43 @@ static void intel_dsi_prepare(struct intel_encoder
*intel_encoder)
struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
struct drm_display_mode *adjusted_mode =
&intel_crtc->config.adjusted_mode;
- enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
+ enum port port;
unsigned int bpp = intel_crtc->config.pipe_bpp;
u32 val, tmp;
+ u16 mode_hdisplay;
DRM_DEBUG_KMS("pipe %c\n", pipe_name(intel_crtc->pipe));
- /* escape clock divider, 20MHz, shared for A and C. device ready must be
-* off when doing this! txclkesc? */
- tmp = I915_READ(MIPI_CTRL(PORT_A));
- tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
- I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
-
- /* read request priority is per pipe */
- tmp = I915_READ(MIPI_CTRL(port));
- tmp &= ~READ_REQUEST_PRIORITY_MASK;
- I915_WRITE(MIPI_CTRL(port), tmp | READ_REQUEST_PRIORITY_HIGH);
+ mode_hdisplay = adjusted_mode->hdisplay;
- /* XXX: why here, why like this? handling in irq handler?! */
- I915_WRITE(MIPI_INTR_STAT(port), 0x);
- I915_WRITE(MIPI_INTR_EN(port), 0x);
-
- I915_WRITE(MIPI_DPHY_PARAM(port), intel_dsi->dphy_reg);
+ if (intel_dsi->dual_link) {
+ mode_hdisplay /= 2;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
+ mode_hdisplay += intel_dsi->pixel_overlap;
+ }
- I915_WRITE(MIPI_DPI_RESOLUTION(port),
- adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT |
- adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT);
+ for_each_dsi_port(port, intel_dsi->ports) {
+ /* escape clock divider, 20MHz, shared for A and C.
+* device ready must be off when doing this! txclkesc? */
+ tmp = I915_READ(MIPI_CTRL(PORT_A));
+ tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+ I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
+
+ /* read request priority is per pipe */
+ tmp = I915_READ(MIPI_CTRL(port));
+ tmp &= ~READ_REQUEST_PRIORITY_MASK;
+ I915_WRITE(MIPI_CTRL(port), tmp | READ_REQUEST_PRIORITY_HIGH);
+
+ /* XXX: why here, why like this? handling in irq handler?! */
+ I915_WRITE(MIPI_INTR_STAT(port), 0x);
+ I915_WRITE(MIPI_INTR_EN(port), 0x);
+
+
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5)
On Fri, Dec 05, 2014 at 10:15:07AM +0200, Ander Conselvan de Oliveira wrote: > Reviewed-by: Ander Conselvan de Oliveira > > > On 12/04/2014 08:27 PM, Matt Roper wrote: > > If we extend the commit_plane handlers for each plane type to be able to > > handle fb=0, then we can easily implement plane disable via the > > update_plane handler. The cursor plane already works this way, and this > > is the direction we need to go to integrate with the atomic plane > > handler. We can now kill off the type-specific disable functions, as > > well as the redundant intel_plane_disable() (not to be confused with > > intel_disable_plane()). > > > > Note that prepare_plane_fb() only gets called as part of update_plane > > when fb!=NULL (by design, to match the semantics of the atomic plane > > helpers); this means that our commit_plane handlers need to handle the > > frontbuffer tracking for the disable case, even though they don't handle > > it for normal updates. > > > > v2: > > - Change BUG_ON to WARN_ON (Ander/Daniel) > > > > v3: > > - Drop unnecessary plane->crtc check since a previous patch to plane > > update ensures that plane->crtc will always be non-NULL, even for > > disable calls that might pass NULL from userspace. (Ander) > > - Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander) > > > > v4: > > - Fix missing whitespace (Ander) > > > > v5: > > - Use state's crtc rather than plane's crtc in > > intel_check_primary_plane(). plane->crtc could be NULL, but we've > > already fixed up state->crtc to ensure it's non-NULL (even if > > userspace passed it as NULL during a disable call). (Ander) > > > > Signed-off-by: Matt Roper > > Reviewed-by: Ander Conselvan de Oliveira > > Ok, merged all 10 patches to dinq, thanks. There was a minor conflict in patch 8 due to the slight changes in patch 7, but nothing big really. -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 5/5] drm/i915: Dual link needs Shutdown and Turn on packet for both ports
For dual link MIPI panels, SHUTDOWN packet needs to send to both Ports
A & C during MIPI encoder disabling sequence. Similarly, TURN ON packet
to be sent to both Ports during MIPI encoder enabling sequence.
v2: Address review comments by Jani
- Used a for loop instead of do-while loop.
v3: Used for_each_dsi_port macro instead of for loop
Signed-off-by: Gaurav K Singh
Signed-off-by: Shobhit Kumar
Reviewed-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dsi_cmd.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c
b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index 8e30684..e97fb6a 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -385,8 +385,7 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool
hs)
struct drm_encoder *encoder = &intel_dsi->base.base;
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
- enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
+ enum port port;
u32 mask;
/* XXX: pipe, hs */
@@ -395,18 +394,23 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd,
bool hs)
else
cmd |= DPI_LP_MODE;
- /* clear bit */
- I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
+ for_each_dsi_port(port, intel_dsi->ports) {
+ /* clear bit */
+ I915_WRITE(MIPI_INTR_STAT(port), SPL_PKT_SENT_INTERRUPT);
- /* XXX: old code skips write if control unchanged */
- if (cmd == I915_READ(MIPI_DPI_CONTROL(port)))
- DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
+ /* XXX: old code skips write if control unchanged */
+ if (cmd == I915_READ(MIPI_DPI_CONTROL(port)))
+ DRM_ERROR("Same special packet %02x twice in a row.\n",
+ cmd);
- I915_WRITE(MIPI_DPI_CONTROL(port), cmd);
+ I915_WRITE(MIPI_DPI_CONTROL(port), cmd);
- mask = SPL_PKT_SENT_INTERRUPT;
- if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask, 100))
- DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
+ mask = SPL_PKT_SENT_INTERRUPT;
+ if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & mask) == mask,
+100))
+ DRM_ERROR("Video mode command 0x%08x send failed.\n",
+ cmd);
+ }
return 0;
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Additional request structure tracing
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) -Summary- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK 366/366 366/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt_kms_force_connector NRUN(3, M35M22)PASS(1, M35) NRUN(1, M35) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M4) HSW igt_kms_force_connector NRUN(3, M40M19M20)PASS(1, M40) NRUN(1, M20) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't complain about stolen conflicts on gen3
On Fri, 11 Apr 2014 15:55:17 +0200
Daniel Vetter wrote:
> Apparently stuff works that way on those machines.
>
> I agree with Chris' concern that this is a bit risky but imo worth a
> shot in -next just for fun. Afaics all these machines have the pci
> resources allocated like that by the BIOS, so I suspect that it's all
> ok.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76983
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71031
> Tested-by: lu hua
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem_stolen.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c index
> 62ef55ba061c..99d147af173a 100644 ---
> a/drivers/gpu/drm/i915/i915_gem_stolen.c +++
> b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -93,7 +93,11 @@ static
> unsigned long i915_stolen_to_physical(struct drm_device *dev) r =
> devm_request_mem_region(dev->dev, base + 1, dev_priv->gtt.stolen_size
> - 1, "Graphics Stolen Memory");
> - if (r == NULL) {
> + /*
> + * GEN3 firmware likes to smash pci bridges into the
> stolen
> + * range. Apparently this works.
> + */
> + if (r == NULL && !IS_GEN3(dev)) {
> DRM_ERROR("conflict detected with stolen
> region: [0x%08x - 0x%08x]\n", base, base +
> (uint32_t)dev_priv->gtt.stolen_size); base = 0;
Yeah just to allay fears: the decode priority on the GMCH is fixed and
specific. The stolen range is demarcated by some regs which the GMCH
decodes before it tries going out into PCI space. So it's safe to see
the stolen range under the bus0 window (probably even under some device
window down the range) but does make things messier for us.
Reviewed-by: Jesse Barnes
Looks like the reporter gave a t-b too.
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
John Harrison writes: > This is already part of the seqno/request patch series and has been > right from the start. See email 'drm/i915: Zero fill the request structure'. Indeed. Sorry for missing this one. My patch can be ignored. But do you agree on failure path? -Mika > On 05/12/2014 17:54, Mika Kuoppala wrote: >> Otherwise we might end up referencing uninitialized fields. >> This is apparent when we try to cleanup the preallocated request >> on ring reset, before any request has been submitted to the ring. >> The request->ctx is foobar and we end up freeing the foobarness. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 >> Cc: John Harrison >> Signed-off-by: Mika Kuoppala >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c >> b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 79b4ca5..2c6c6f8 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) >> if (ring->outstanding_lazy_request) >> return 0; >> >> -request = kmalloc(sizeof(*request), GFP_KERNEL); >> +request = kzalloc(sizeof(*request), GFP_KERNEL); >> if (request == NULL) >> return -ENOMEM; >> ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
But yes, the reasoning why it crashes without the zero fill is correct. Dodgy context pointers that used to be ignored now get processed. Doing the zero fill keeps it all sane. On 05/12/2014 17:54, John Harrison wrote: This is already part of the seqno/request patch series and has been right from the start. See email 'drm/i915: Zero fill the request structure'. On 05/12/2014 17:54, Mika Kuoppala wrote: Otherwise we might end up referencing uninitialized fields. This is apparent when we try to cleanup the preallocated request on ring reset, before any request has been submitted to the ring. The request->ctx is foobar and we end up freeing the foobarness. References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 Cc: John Harrison Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 79b4ca5..2c6c6f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) if (ring->outstanding_lazy_request) return 0; -request = kmalloc(sizeof(*request), GFP_KERNEL); +request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
This is already part of the seqno/request patch series and has been right from the start. See email 'drm/i915: Zero fill the request structure'. On 05/12/2014 17:54, Mika Kuoppala wrote: Otherwise we might end up referencing uninitialized fields. This is apparent when we try to cleanup the preallocated request on ring reset, before any request has been submitted to the ring. The request->ctx is foobar and we end up freeing the foobarness. References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 Cc: John Harrison Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 79b4ca5..2c6c6f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) if (ring->outstanding_lazy_request) return 0; - request = kmalloc(sizeof(*request), GFP_KERNEL); + request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Clean the request structure on alloc
Otherwise we might end up referencing uninitialized fields. This is apparent when we try to cleanup the preallocated request on ring reset, before any request has been submitted to the ring. The request->ctx is foobar and we end up freeing the foobarness. References: https://bugs.freedesktop.org/show_bug.cgi?id=86959 References: https://bugs.freedesktop.org/show_bug.cgi?id=86962 References: https://bugs.freedesktop.org/show_bug.cgi?id=86992 Cc: John Harrison Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 79b4ca5..2c6c6f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) if (ring->outstanding_lazy_request) return 0; - request = kmalloc(sizeof(*request), GFP_KERNEL); + request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 05/12/2014 17:36, Jani Nikula wrote:
On Fri, 05 Dec 2014, "Siluvery, Arun" wrote:
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock. Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
From the host perspective a dual link (or dual channel) DSI device is
two independent peripheral devices. On the peripheral side the display
has to combine the input from the two links (which may be two
independent DSI blocks on the peripheral as well) into one contiguous
display. I don't know the details, but I'm guessing pixel overlap just
makes it easier for the peripheral implementation to get it all
together.
Thank you for the details.
I am just wondering how few extra pixels help on the display side unless
they are fixed values which act like some kind of markers to synchronize
between two halves.
regards
Arun
BR,
Jani.
regards
Arun
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP(1<<31)
#define VLV_PWRDWNUPCTL 0xA294
+#define VLV_CHICKEN_3 0x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT 30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not been updated
in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR 0x44020
#define GEN6_PMIMR 0x44024 /* rps_lock */
#define GEN6_PMIIR 0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT 2
u16 dual_link:2;
u16 lane_cnt:2;
- u16 rsvd3:12;
+ u16 pixel_overlap:3;
+ u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
enum port port;
u32 temp;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ temp = I915_READ(VLV_CHICKEN_3);
+ temp &= ~PIXEL_OVERLAP_CNT_MASK |
+ intel_dsi->pixel_overlap <<
+ PIXEL_OVERLAP_CNT_SHIFT;
+ I915_WRITE(VLV_CHICKEN_3, temp);
+ }
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE 0
+#define DSI_DUAL_LINK_FRONT_BACK 1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+ u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool ge
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib: Add swap() macro
On Fri, 05 Dec 2014, Ville Syrjälä wrote:
> On Fri, Dec 05, 2014 at 06:44:19PM +0200, Jani Nikula wrote:
>> On Fri, 05 Dec 2014, ville.syrj...@linux.intel.com wrote:
>> > From: Ville Syrjälä
>> >
>> > swap() will swap its two arguments while keeping the required
>> > tmp variable hidden. Makes for neater code.
>> >
>> > Signed-off-by: Ville Syrjälä
>> > ---
>> > lib/igt_aux.h | 6 ++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>> > index 6c83c53..63e1b06 100644
>> > --- a/lib/igt_aux.h
>> > +++ b/lib/igt_aux.h
>> > @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t
>> > size, unsigned mode);
>> > #define min(a, b) ((a) < (b) ? (a) : (b))
>> > #define max(a, b) ((a) > (b) ? (a) : (b))
>> >
>> > +#define swap(a, b) do { \
>> > + typeof(a) _tmp = (a); \
>> > + (a) = (b); \
>> > + (b) = _tmp; \
>> > +} while (0)
>>
>> Nitpick, make the macro take pointers instead, and amend the cocci patch
>> accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
>> obvious than swap(a, b)...
>
> But then it's different than the kernel swap() we all love.
Fair enough. I'll cry a little. I'll feel better soon.
>
>>
>> Jani.
>>
>>
>> > +
>> > #endif /* IGT_AUX_H */
>> > --
>> > 2.0.4
>> >
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Ville Syrjälä
> Intel OTC
--
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 04/10] drm/i915: Pixel Clock changes for DSI dual link
On Fri, 05 Dec 2014, "Siluvery, Arun" wrote:
> On 05/12/2014 16:33, Singh, Gaurav K wrote:
>>
>> On 12/4/2014 2:57 PM, Jani Nikula wrote:
>>> On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock. Pixel
overlap
can be enabled if needed by panel, then in that case, pixel clock will be
increased for extra pixels.
>
> just a question, why do we need pixel overlap?
> I couldn't find more details from spec other than that when overlap is
> set some extra pixels are sent.
From the host perspective a dual link (or dual channel) DSI device is
two independent peripheral devices. On the peripheral side the display
has to combine the input from the two links (which may be two
independent DSI blocks on the peripheral as well) into one contiguous
display. I don't know the details, but I'm guessing pixel overlap just
makes it easier for the peripheral implementation to get it all
together.
BR,
Jani.
>
> regards
> Arun
>
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP(1<<31)
#define VLV_PWRDWNUPCTL 0xA294
+#define VLV_CHICKEN_3 0x7040C
+#define PIXEL_OVERLAP_CNT_MASK (3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT 30
>>> I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
>>>
>>> Given that I can't find the register my review is pretty shallow, but I
>>> don't spot anything obviously wrong either. With these caveats,
>>>
>>> Reviewed-by: Jani Nikula
>>>
>>> This reg is available in BSpec though the bit definitions have not been
>>> updated in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR 0x44020
#define GEN6_PMIMR 0x44024 /* rps_lock */
#define GEN6_PMIIR 0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT 2
u16 dual_link:2;
u16 lane_cnt:2;
- u16 rsvd3:12;
+ u16 pixel_overlap:3;
+ u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct
intel_encoder *encoder)
enum port port;
u32 temp;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ temp = I915_READ(VLV_CHICKEN_3);
+ temp &= ~PIXEL_OVERLAP_CNT_MASK |
+ intel_dsi->pixel_overlap <<
+ PIXEL_OVERLAP_CNT_SHIFT;
+ I915_WRITE(VLV_CHICKEN_3, temp);
+ }
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE0
+#define DSI_DUAL_LINK_FRONT_BACK 1
+#define DSI_DUAL_LINK_PIXEL_ALT 2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+ u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/5/2014 10:24 PM, Siluvery, Arun wrote:
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock.
Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock
will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
regards
Arun
In Front-Back video mode for MIPI Dual Link the first half of columns of
pixel is always transmitted by MIPI Port A and second half in MIPI Port C.
Pixel Overlap Count sets the number of Pixels to be overlapped per half
of Scanline in Front-Back video mode. In dual link front-back mode,
pixel overlap count needs to be added to HActive to account for the
additional pixels added due to overlap.
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21
+
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP(1<<31)
#define VLV_PWRDWNUPCTL0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not
been updated in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT2
u16 dual_link:2;
u16 lane_cnt:2;
-u16 rsvd3:12;
+u16 pixel_overlap:3;
+u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c
b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct
intel_encoder *encoder)
enum port port;
u32 temp;
+if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+temp = I915_READ(VLV_CHICKEN_3);
+temp &= ~PIXEL_OVERLAP_CNT_MASK |
+intel_dsi->pixel_overlap <<
+PIXEL_OVERLAP_CNT_SHIFT;
+I915_WRITE(VLV_CHICKEN_3, temp);
+}
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE0
+#define DSI_DUAL_LINK_FRONT_BACK1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool generic_init(struct
intel_dsi_device *dsi)
intel_dsi->lane_count = mipi_config->lane_cnt + 1;
intel_dsi->pixel_format =
mipi_config->videomode_color_format << 7;
intel_dsi->dual_link = mipi_config->dual_link;
+intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
if (intel_dsi->dual_link)
intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
@@ -310,6 +311,20 @@ static bool generic_init(struct
intel_dsi_device *dsi)
Re: [Intel-gfx] [PATCH] drm/i915: allow requesting the audio power well for all platforms
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) -Summary- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -2 366/366 364/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible DMESG_WARN(1, M26)PASS(3, M37M26) NSPT(1, M26) *ILK igt_kms_flip_wf_vblank-vs-modeset-interruptible PASS(2, M37M26) DMESG_WARN(1, M26) SNB igt_kms_force_connector NRUN(3, M35M22)PASS(1, M35) NRUN(1, M22) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(13, M4M34M21) PASS(1, M21) HSW igt_kms_force_connector NRUN(3, M40M19M20)PASS(1, M40) NRUN(1, M19) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib: Add swap() macro
On Fri, Dec 05, 2014 at 06:44:19PM +0200, Jani Nikula wrote:
> On Fri, 05 Dec 2014, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > swap() will swap its two arguments while keeping the required
> > tmp variable hidden. Makes for neater code.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > lib/igt_aux.h | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index 6c83c53..63e1b06 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size,
> > unsigned mode);
> > #define min(a, b) ((a) < (b) ? (a) : (b))
> > #define max(a, b) ((a) > (b) ? (a) : (b))
> >
> > +#define swap(a, b) do {\
> > + typeof(a) _tmp = (a); \
> > + (a) = (b); \
> > + (b) = _tmp; \
> > +} while (0)
>
> Nitpick, make the macro take pointers instead, and amend the cocci patch
> accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
> obvious than swap(a, b)...
But then it's different than the kernel swap() we all love.
>
> Jani.
>
>
> > +
> > #endif /* IGT_AUX_H */
> > --
> > 2.0.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 04:26:23PM +, Chris Wilson wrote:
> On Fri, Dec 05, 2014 at 04:53:49PM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> > > On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > > > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > > > Currently we initialise the rings, add the first context switch to the
> > > > > ring and execute our golden state then enable (aliasing or full)
> > > > > ppgtt.
> > > > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > > > MI_LRI, we end up executing the context switch and golden render state
> > > > > with an invalid PD generating page faults. To solve this issue, first
> > > > > do
> > > > > the ppgtt PD setup, then set the default context and write the
> > > > > commands
> > > > > to run the render state into the ring, before we activate the ring.
> > > > > This
> > > > > allows us to be sure that the register state is valid before we begin
> > > > > execution.
> > > > >
> > > > > This was spotted when writing the seqno/request conversion, but only
> > > > > with
> > > > > the ERROR capture did I realise that it was a necessity now.
> > > > >
> > > > > RFC: cleanup the error handling in i915_gem_init_hw.
> > > > >
> > > > > Signed-off-by: Chris Wilson
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > > index c1c11418231b..c13842d3cbc9 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > > > >*/
> > > > > init_unused_rings(dev);
> > > > >
> > > > > - for_each_ring(ring, dev_priv, i) {
> > > > > - ret = ring->init_hw(ring);
> > > > > - if (ret)
> > > > > - return ret;
> > > > > - }
> > > > > -
> > > > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> > > >
> > > > This is going to assume ring->head/tail are already valid?
> > >
> > > We write into the ring obj, not the ring itself, which should be setup
> > > during the various intel_init_engine, i.e. the backing storage is
> > > independent of the actual registers.
> >
> > I mean the software shadows, not the registers themselves. When the GPU
> > hangs I expect rign->head != ring->tail. So what makes those two identical
> > again after the GPU reset?
>
> Why would they be equal after reset?
They wouldn't. That's my whole point.
> At the moment, we discard all
> outstanding requests which makes them equal.
OK, so you're saying somehwere we set them to some sane values. But
I don't see any obvious code for that. The obvious code was in
init_ring_common(), but you just removed said code in this patch.
So unless I'm totally misreading the code, ->head and ->tail will be
left at their pre-reset values, then i915_gem_l3_remap() will pile on
some new stuff at ->tail, and then later you restart the ring from
->head, which would still include all the discarded junk from before
the reset.
> We have been discussing the
> need to execute pending batches from non-guitly Batches for yonks, in
> which case we would not expect ring->head == ring->tail after a reset.
Yeah, but let's not confuse the discussion with advanced topics.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 05/12/2014 16:33, Singh, Gaurav K wrote:
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock. Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock will be
increased for extra pixels.
just a question, why do we need pixel overlap?
I couldn't find more details from spec other than that when overlap is
set some extra pixels are sent.
regards
Arun
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31)
#define VLV_PWRDWNUPCTL 0xA294
+#define VLV_CHICKEN_3 0x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT 30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not been updated
in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR 0x44020
#define GEN6_PMIMR 0x44024 /* rps_lock */
#define GEN6_PMIIR 0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT 2
u16 dual_link:2;
u16 lane_cnt:2;
- u16 rsvd3:12;
+ u16 pixel_overlap:3;
+ u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
enum port port;
u32 temp;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ temp = I915_READ(VLV_CHICKEN_3);
+ temp &= ~PIXEL_OVERLAP_CNT_MASK |
+ intel_dsi->pixel_overlap <<
+ PIXEL_OVERLAP_CNT_SHIFT;
+ I915_WRITE(VLV_CHICKEN_3, temp);
+ }
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE 0
+#define DSI_DUAL_LINK_FRONT_BACK 1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+ u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
intel_dsi->lane_count = mipi_config->lane_cnt + 1;
intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
intel_dsi->dual_link = mipi_config->dual_link;
+ intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
if (intel_dsi->dual_link)
intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
@@ -310,6 +311,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
pclk = mode->clock;
+ /* In dual link mode each port needs half of pixel clock */
+ if (intel_dsi->dual_link) {
+ pclk = pclk / 2;
+
+ /* we can enable pixel_overlap if needed by panel. In this
+* case we ne
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib: Add swap() macro
On Fri, 05 Dec 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> swap() will swap its two arguments while keeping the required
> tmp variable hidden. Makes for neater code.
>
> Signed-off-by: Ville Syrjälä
> ---
> lib/igt_aux.h | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6c83c53..63e1b06 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size,
> unsigned mode);
> #define min(a, b) ((a) < (b) ? (a) : (b))
> #define max(a, b) ((a) > (b) ? (a) : (b))
>
> +#define swap(a, b) do { \
> + typeof(a) _tmp = (a); \
> + (a) = (b); \
> + (b) = _tmp; \
> +} while (0)
Nitpick, make the macro take pointers instead, and amend the cocci patch
accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
obvious than swap(a, b)...
Jani.
> +
> #endif /* IGT_AUX_H */
> --
> 2.0.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 04/10] drm/i915: Pixel Clock changes for DSI dual link
On 12/4/2014 2:57 PM, Jani Nikula wrote:
On Thu, 04 Dec 2014, Gaurav K Singh wrote:
For dual link MIPI Panels, each port needs half of pixel clock. Pixel overlap
can be enabled if needed by panel, then in that case, pixel clock will be
increased for extra pixels.
v2 : Address review comments by Jani
- Removed the bit mask used for ->dual_link
- Used DSI instead of MIPI for #define variables
Signed-off-by: Gaurav K Singh
---
drivers/gpu/drm/i915/i915_reg.h|4
drivers/gpu/drm/i915/intel_bios.h |3 ++-
drivers/gpu/drm/i915/intel_dsi.c |8
drivers/gpu/drm/i915/intel_dsi.h |6 ++
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 21 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c981f5d..87149ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6029,6 +6029,10 @@ enum punit_power_well {
#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31)
#define VLV_PWRDWNUPCTL 0xA294
+#define VLV_CHICKEN_30x7040C
+#define PIXEL_OVERLAP_CNT_MASK(3 << 30)
+#define PIXEL_OVERLAP_CNT_SHIFT 30
I didn't find this register, but does it not need + VLV_DISPLAY_BASE?
Given that I can't find the register my review is pretty shallow, but I
don't spot anything obviously wrong either. With these caveats,
Reviewed-by: Jani Nikula
This reg is available in BSpec though the bit definitions have not been updated
in the BSpec. Also, it was communicated by the BIOS team.
+
#define GEN6_PMISR0x44020
#define GEN6_PMIMR0x44024 /* rps_lock */
#define GEN6_PMIIR0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index de01167..a6a8710 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -818,7 +818,8 @@ struct mipi_config {
#define DUAL_LINK_PIXEL_ALT 2
u16 dual_link:2;
u16 lane_cnt:2;
- u16 rsvd3:12;
+ u16 pixel_overlap:3;
+ u16 rsvd3:9;
u16 rsvd4;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index dbe52e9..4e18abd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -111,6 +111,14 @@ static void intel_dsi_port_enable(struct intel_encoder
*encoder)
enum port port;
u32 temp;
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ temp = I915_READ(VLV_CHICKEN_3);
+ temp &= ~PIXEL_OVERLAP_CNT_MASK |
+ intel_dsi->pixel_overlap <<
+ PIXEL_OVERLAP_CNT_SHIFT;
+ I915_WRITE(VLV_CHICKEN_3, temp);
+ }
+
for_each_dsi_port(port, intel_dsi->ports) {
temp = I915_READ(MIPI_PORT_CTRL(port));
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index f2cc2fc..8fe2064 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
#include
#include "intel_drv.h"
+/* Dual Link support */
+#define DSI_DUAL_LINK_NONE 0
+#define DSI_DUAL_LINK_FRONT_BACK 1
+#define DSI_DUAL_LINK_PIXEL_ALT2
+
struct intel_dsi_device {
unsigned int panel_id;
const char *name;
@@ -105,6 +110,7 @@ struct intel_dsi {
u8 escape_clk_div;
u8 dual_link;
+ u8 pixel_overlap;
u32 port_bits;
u32 bw_timer;
u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f60146f..f8c2269 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -288,6 +288,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
intel_dsi->lane_count = mipi_config->lane_cnt + 1;
intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
intel_dsi->dual_link = mipi_config->dual_link;
+ intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
if (intel_dsi->dual_link)
intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
@@ -310,6 +311,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
pclk = mode->clock;
+ /* In dual link mode each port needs half of pixel clock */
+ if (intel_dsi->dual_link) {
+ pclk = pclk / 2;
+
+ /* we can enable pixel_overlap if needed by panel. In this
+* case we need to increase the pixelclock for extra pixels
+*/
+ if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+ pclk += DIV_ROUND_UP(mode->vtotal *
+ intel
Re: [Intel-gfx] [PATCH] igt/drm_read: Abuse read(drm)
On Fri, Dec 05, 2014 at 03:46:54PM +0100, Daniel Vetter wrote:
> On Fri, Dec 05, 2014 at 01:14:40PM +, Chris Wilson wrote:
> > +static void assert_empty(int fd)
> > +{
> > + struct pollfd pfd = {fd, POLLIN };
> > + igt_assert(poll(&pfd, 1, 0) == 0);
> > +}
> > +
> > +static void generate_event(int fd)
> > +{
> > + union drm_wait_vblank vbl;
> > +
> > + /* We assume that pipe 0 is running */
>
> This assumption falls short on gen2/3 with lvds only, and might fall short
> if a previous test wreaked havoc. Also dpms tends to foul up kms tests,
> too. I think we do need the full-blown modeset dance here unfortunately.
I actually hoped that queuing a .relative=0 vblank event the kernel would
have seen that it was a no-op and just put the event into the queue, but
it still does the vblank_get/_put and so requires the active pipe.
Oh well, it's not too complicated to setup a single pipe for my purpose.
-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 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 04:53:49PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> > On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > > Currently we initialise the rings, add the first context switch to the
> > > > ring and execute our golden state then enable (aliasing or full) ppgtt.
> > > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > > MI_LRI, we end up executing the context switch and golden render state
> > > > with an invalid PD generating page faults. To solve this issue, first do
> > > > the ppgtt PD setup, then set the default context and write the commands
> > > > to run the render state into the ring, before we activate the ring. This
> > > > allows us to be sure that the register state is valid before we begin
> > > > execution.
> > > >
> > > > This was spotted when writing the seqno/request conversion, but only
> > > > with
> > > > the ERROR capture did I realise that it was a necessity now.
> > > >
> > > > RFC: cleanup the error handling in i915_gem_init_hw.
> > > >
> > > > Signed-off-by: Chris Wilson
> > > > ---
> > > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > index c1c11418231b..c13842d3cbc9 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > > > */
> > > > init_unused_rings(dev);
> > > >
> > > > - for_each_ring(ring, dev_priv, i) {
> > > > - ret = ring->init_hw(ring);
> > > > - if (ret)
> > > > - return ret;
> > > > - }
> > > > -
> > > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> > >
> > > This is going to assume ring->head/tail are already valid?
> >
> > We write into the ring obj, not the ring itself, which should be setup
> > during the various intel_init_engine, i.e. the backing storage is
> > independent of the actual registers.
>
> I mean the software shadows, not the registers themselves. When the GPU
> hangs I expect rign->head != ring->tail. So what makes those two identical
> again after the GPU reset?
Why would they be equal after reset? At the moment, we discard all
outstanding requests which makes them equal. We have been discussing the
need to execute pending batches from non-guitly Batches for yonks, in
which case we would not expect ring->head == ring->tail after a reset.
-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 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 03:58:40PM +0100, Daniel Vetter wrote:
> On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> > On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > > Currently we initialise the rings, add the first context switch to the
> > > > ring and execute our golden state then enable (aliasing or full) ppgtt.
> > > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > > MI_LRI, we end up executing the context switch and golden render state
> > > > with an invalid PD generating page faults. To solve this issue, first do
> > > > the ppgtt PD setup, then set the default context and write the commands
> > > > to run the render state into the ring, before we activate the ring. This
> > > > allows us to be sure that the register state is valid before we begin
> > > > execution.
> > > >
> > > > This was spotted when writing the seqno/request conversion, but only
> > > > with
> > > > the ERROR capture did I realise that it was a necessity now.
> > > >
> > > > RFC: cleanup the error handling in i915_gem_init_hw.
> > > >
> > > > Signed-off-by: Chris Wilson
> > > > ---
> > > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > index c1c11418231b..c13842d3cbc9 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > > > */
> > > > init_unused_rings(dev);
> > > >
> > > > - for_each_ring(ring, dev_priv, i) {
> > > > - ret = ring->init_hw(ring);
> > > > - if (ret)
> > > > - return ret;
> > > > - }
> > > > -
> > > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> > >
> > > This is going to assume ring->head/tail are already valid?
> >
> > We write into the ring obj, not the ring itself, which should be setup
> > during the various intel_init_engine, i.e. the backing storage is
> > independent of the actual registers.
>
> But there's still intel_ring_advance which calls ->write_tail all over the
> place. So we drop all these mmio writes into nirvana since we'll reset the
> ring later on?
intel_ring_advance() doesn't do the register update, it just updates
ring->tail. And even if it did, whilst the ring is disabled, nobody is
listening, right?
-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 i-g-t v2 3/4] tests/kms_flip: Calibrate the dummy load delay in kms_flip
From: Ville Syrjälä
Try to tune the dummy load to ~1 second. The calibration happens the
first time dummy load is generated.
v2: Actually do the number of ops intended and
calibrate to 1 second and not 2
Signed-off-by: Ville Syrjälä
---
tests/kms_flip.c | 81
1 file changed, 70 insertions(+), 11 deletions(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index f2b5e2b..3d54718 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -84,6 +84,9 @@
#define DRM_CAP_TIMESTAMP_MONOTONIC 6
#endif
+#define USEC_PER_SEC 100L
+#define NSEC_PER_SEC 10L
+
drmModeRes *resources;
int drm_fd;
static drm_intel_bufmgr *bufmgr;
@@ -162,6 +165,34 @@ static unsigned long gettime_us(void)
return ts.tv_sec * 100 + ts.tv_nsec / 1000;
}
+static int calibrate_dummy_load(struct test_output *o,
+ const char *ring_name,
+ int (*emit)(struct test_output *o, int limit,
int timeout))
+{
+ unsigned long start;
+ int ops = 1;
+
+ start = gettime_us();
+
+ do {
+ unsigned long diff;
+ int ret;
+
+ ret = emit(o, (ops+1)/2, 10);
+ diff = gettime_us() - start;
+
+ if (ret || diff / USEC_PER_SEC >= 1)
+ break;
+
+ ops += ops;
+ } while (ops < 10);
+
+ igt_debug("%s dummy load calibrated: %d operations / second\n",
+ ring_name, ops);
+
+ return ops;
+}
+
static void blit_copy(drm_intel_bo *dst, drm_intel_bo *src,
unsigned int width, unsigned int height,
unsigned int dst_pitch, unsigned int src_pitch)
@@ -187,14 +218,12 @@ static void blit_copy(drm_intel_bo *dst, drm_intel_bo
*src,
}
}
-static void emit_dummy_load__bcs(struct test_output *o)
+static int _emit_dummy_load__bcs(struct test_output *o, int limit, int timeout)
{
- int i, limit;
+ int i, ret = 0;
drm_intel_bo *src_bo, *dst_bo, *fb_bo;
struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
- limit = intel_gen(devid) < 6 ? 500 : 5000;
-
src_bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(src_bo);
@@ -216,9 +245,25 @@ static void emit_dummy_load__bcs(struct test_output *o)
fb_info->stride, 2048*4);
intel_batchbuffer_flush(batch);
+ if (timeout > 0)
+ ret = drm_intel_gem_bo_wait(fb_bo, timeout * NSEC_PER_SEC);
+
drm_intel_bo_unreference(src_bo);
drm_intel_bo_unreference(dst_bo);
drm_intel_bo_unreference(fb_bo);
+
+ return ret;
+}
+
+static void emit_dummy_load__bcs(struct test_output *o, int seconds)
+{
+ static int ops_per_sec;
+
+ if (ops_per_sec == 0)
+ ops_per_sec = calibrate_dummy_load(o, "bcs",
+ _emit_dummy_load__bcs);
+
+ _emit_dummy_load__bcs(o, seconds * ops_per_sec, 0);
}
static void emit_fence_stress(struct test_output *o)
@@ -263,18 +308,16 @@ static void emit_fence_stress(struct test_output *o)
free(exec);
}
-static void emit_dummy_load__rcs(struct test_output *o)
+static int _emit_dummy_load__rcs(struct test_output *o, int limit, int timeout)
{
const struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
igt_render_copyfunc_t copyfunc;
struct igt_buf sb[3], *src, *dst, *fb;
- int i, limit;
+ int i, ret = 0;
copyfunc = igt_get_render_copyfunc(devid);
if (copyfunc == NULL)
- return emit_dummy_load__bcs(o);
-
- limit = intel_gen(devid) < 6 ? 500 : 5000;
+ return _emit_dummy_load__bcs(o, limit, timeout);
sb[0].bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(sb[0].bo);
@@ -318,9 +361,25 @@ static void emit_dummy_load__rcs(struct test_output *o)
fb, 0, 0);
intel_batchbuffer_flush(batch);
+ if (timeout > 0)
+ ret = drm_intel_gem_bo_wait(fb->bo, timeout * NSEC_PER_SEC);
+
drm_intel_bo_unreference(sb[0].bo);
drm_intel_bo_unreference(sb[1].bo);
drm_intel_bo_unreference(sb[2].bo);
+
+ return ret;
+}
+
+static void emit_dummy_load__rcs(struct test_output *o, int seconds)
+{
+ static int ops_per_sec;
+
+ if (ops_per_sec == 0)
+ ops_per_sec = calibrate_dummy_load(o, "rcs",
+ _emit_dummy_load__rcs);
+
+ _emit_dummy_load__bcs(o, seconds * ops_per_sec, 0);
}
static void dpms_off_other_outputs(struct test_output *o)
@@ -820,10 +879,10 @@ static unsigned int run_test_step(struct test_output *o)
dpms_off_other_outputs(o);
if (o->flags & TEST_WITH_DUMMY_BCS)
- emit_dummy_load__bcs(o);
+ emit_dummy_load__bcs(o, 1);
[Intel-gfx] [PATCH i-g-t 2/4] tests/kms_flip: Use fixed size (2kx2k) buffers for dummy load
From: Ville Syrjälä
Make the dummy load independent of the display resolution by using a
two fixed size dummy bos to generate the load. As a final step do
another copy from one of the dummy bos to the fb to make sure there's
a dependency between the dummy load and any subsequent operation on
the fb.
Signed-off-by: Ville Syrjälä
---
tests/kms_flip.c | 59 ++--
1 file changed, 40 insertions(+), 19 deletions(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 8b401be..f2b5e2b 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -190,28 +190,35 @@ static void blit_copy(drm_intel_bo *dst, drm_intel_bo
*src,
static void emit_dummy_load__bcs(struct test_output *o)
{
int i, limit;
- drm_intel_bo *dummy_bo, *target_bo;
+ drm_intel_bo *src_bo, *dst_bo, *fb_bo;
struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
- unsigned pitch = fb_info->stride;
limit = intel_gen(devid) < 6 ? 500 : 5000;
- dummy_bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", fb_info->size, 4096);
- igt_assert(dummy_bo);
- target_bo = gem_handle_to_libdrm_bo(bufmgr, drm_fd, "imported",
fb_info->gem_handle);
- igt_assert(target_bo);
+ src_bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
+ igt_assert(src_bo);
+
+ dst_bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
+ igt_assert(dst_bo);
+
+ fb_bo = gem_handle_to_libdrm_bo(bufmgr, drm_fd, "imported",
fb_info->gem_handle);
+ igt_assert(fb_bo);
for (i = 0; i < limit; i++) {
- blit_copy(dummy_bo, target_bo,
- o->fb_width, o->fb_height,
- pitch, pitch);
+ blit_copy(dst_bo, src_bo,
+ 2048, 2048,
+ 2048*4, 2048*4);
- swap(dummy_bo, target_bo);
+ swap(src_bo, dst_bo);
}
+ blit_copy(fb_bo, src_bo,
+ min(o->fb_width, 2048), min(o->fb_height, 2048),
+ fb_info->stride, 2048*4);
intel_batchbuffer_flush(batch);
- drm_intel_bo_unreference(dummy_bo);
- drm_intel_bo_unreference(target_bo);
+ drm_intel_bo_unreference(src_bo);
+ drm_intel_bo_unreference(dst_bo);
+ drm_intel_bo_unreference(fb_bo);
}
static void emit_fence_stress(struct test_output *o)
@@ -260,7 +267,7 @@ static void emit_dummy_load__rcs(struct test_output *o)
{
const struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
igt_render_copyfunc_t copyfunc;
- struct igt_buf sb[2], *src, *dst;
+ struct igt_buf sb[3], *src, *dst, *fb;
int i, limit;
copyfunc = igt_get_render_copyfunc(devid);
@@ -269,37 +276,51 @@ static void emit_dummy_load__rcs(struct test_output *o)
limit = intel_gen(devid) < 6 ? 500 : 5000;
- sb[0].bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", fb_info->size, 4096);
+ sb[0].bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(sb[0].bo);
sb[0].size = sb[0].bo->size;
sb[0].tiling = I915_TILING_NONE;
sb[0].data = NULL;
sb[0].num_tiles = sb[0].bo->size;
- sb[0].stride = 4 * o->fb_width;
+ sb[0].stride = 4 * 2048;
- sb[1].bo = gem_handle_to_libdrm_bo(bufmgr, drm_fd, "imported",
fb_info->gem_handle);
+ sb[1].bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(sb[1].bo);
sb[1].size = sb[1].bo->size;
- sb[1].tiling = fb_info->tiling;
+ sb[1].tiling = I915_TILING_NONE;
sb[1].data = NULL;
sb[1].num_tiles = sb[1].bo->size;
- sb[1].stride = fb_info->stride;
+ sb[1].stride = 4 * 2048;
+
+ sb[2].bo = gem_handle_to_libdrm_bo(bufmgr, drm_fd, "imported",
fb_info->gem_handle);
+ igt_assert(sb[2].bo);
+ sb[2].size = sb[2].bo->size;
+ sb[2].tiling = fb_info->tiling;
+ sb[2].data = NULL;
+ sb[2].num_tiles = sb[2].bo->size;
+ sb[2].stride = fb_info->stride;
src = &sb[0];
dst = &sb[1];
+ fb = &sb[2];
for (i = 0; i < limit; i++) {
copyfunc(batch, NULL,
src, 0, 0,
-o->fb_width, o->fb_height,
+2048, 2048,
dst, 0, 0);
swap(src, dst);
}
+ copyfunc(batch, NULL,
+src, 0, 0,
+min(o->fb_width, 2048), min(o->fb_height, 2048),
+fb, 0, 0);
intel_batchbuffer_flush(batch);
drm_intel_bo_unreference(sb[0].bo);
drm_intel_bo_unreference(sb[1].bo);
+ drm_intel_bo_unreference(sb[2].bo);
}
static void dpms_off_other_outputs(struct test_output *o)
--
2.0.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/lis
[Intel-gfx] [PATCH i-g-t 4/4] tests/kms_flip: Target the back buffer with the dummy load
From: Ville Syrjälä Aim the dummy load to the current back buffer instead if the front buffer. Assuming the idea is to get the next flip to be stuck behind the dummy load? Signed-off-by: Ville Syrjälä --- tests/kms_flip.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/kms_flip.c b/tests/kms_flip.c index 5d5302d..f847a86 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -878,15 +878,15 @@ static unsigned int run_test_step(struct test_output *o) if (o->flags & TEST_DPMS_OFF_OTHERS) dpms_off_other_outputs(o); + if (!(o->flags & TEST_SINGLE_BUFFER)) + o->current_fb_id = !o->current_fb_id; + if (o->flags & TEST_WITH_DUMMY_BCS) emit_dummy_load__bcs(o, 1); if (o->flags & TEST_WITH_DUMMY_RCS) emit_dummy_load__rcs(o, 1); - if (!(o->flags & TEST_SINGLE_BUFFER)) - o->current_fb_id = !o->current_fb_id; - if (o->flags & TEST_FB_RECREATE) recreate_fb(o); new_fb_id = o->fb_ids[o->current_fb_id]; -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/4] tests/kms_flip: Calibrate the dummy load delay in kms_flip
From: Ville Syrjälä
Try to tune the dummy load to ~1 second. The calibration happens the
first time dummy load is generated.
Signed-off-by: Ville Syrjälä
---
tests/kms_flip.c | 81
1 file changed, 70 insertions(+), 11 deletions(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index f2b5e2b..5d5302d 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -84,6 +84,9 @@
#define DRM_CAP_TIMESTAMP_MONOTONIC 6
#endif
+#define USEC_PER_SEC 100L
+#define NSEC_PER_SEC 10L
+
drmModeRes *resources;
int drm_fd;
static drm_intel_bufmgr *bufmgr;
@@ -162,6 +165,34 @@ static unsigned long gettime_us(void)
return ts.tv_sec * 100 + ts.tv_nsec / 1000;
}
+static int calibrate_dummy_load(struct test_output *o,
+ const char *ring_name,
+ int (*emit)(struct test_output *o, int limit,
int timeout))
+{
+ unsigned long start;
+ int ops = 1;
+
+ start = gettime_us();
+
+ do {
+ unsigned long diff;
+ int ret;
+
+ ret = emit(o, ops, 10);
+ diff = gettime_us() - start;
+
+ if (ret || diff / USEC_PER_SEC > 1)
+ break;
+
+ ops += ops;
+ } while (ops < 10);
+
+ igt_debug("%s dummy load calibrated: %d operations / second\n",
+ ring_name, ops);
+
+ return ops;
+}
+
static void blit_copy(drm_intel_bo *dst, drm_intel_bo *src,
unsigned int width, unsigned int height,
unsigned int dst_pitch, unsigned int src_pitch)
@@ -187,14 +218,12 @@ static void blit_copy(drm_intel_bo *dst, drm_intel_bo
*src,
}
}
-static void emit_dummy_load__bcs(struct test_output *o)
+static int _emit_dummy_load__bcs(struct test_output *o, int limit, int timeout)
{
- int i, limit;
+ int i, ret = 0;
drm_intel_bo *src_bo, *dst_bo, *fb_bo;
struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
- limit = intel_gen(devid) < 6 ? 500 : 5000;
-
src_bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(src_bo);
@@ -216,9 +245,25 @@ static void emit_dummy_load__bcs(struct test_output *o)
fb_info->stride, 2048*4);
intel_batchbuffer_flush(batch);
+ if (timeout > 0)
+ ret = drm_intel_gem_bo_wait(fb_bo, timeout * NSEC_PER_SEC);
+
drm_intel_bo_unreference(src_bo);
drm_intel_bo_unreference(dst_bo);
drm_intel_bo_unreference(fb_bo);
+
+ return ret;
+}
+
+static void emit_dummy_load__bcs(struct test_output *o, int seconds)
+{
+ static int ops_per_sec;
+
+ if (ops_per_sec == 0)
+ ops_per_sec = calibrate_dummy_load(o, "bcs",
+ _emit_dummy_load__bcs);
+
+ _emit_dummy_load__bcs(o, seconds * ops_per_sec, 0);
}
static void emit_fence_stress(struct test_output *o)
@@ -263,18 +308,16 @@ static void emit_fence_stress(struct test_output *o)
free(exec);
}
-static void emit_dummy_load__rcs(struct test_output *o)
+static int _emit_dummy_load__rcs(struct test_output *o, int limit, int timeout)
{
const struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
igt_render_copyfunc_t copyfunc;
struct igt_buf sb[3], *src, *dst, *fb;
- int i, limit;
+ int i, ret = 0;
copyfunc = igt_get_render_copyfunc(devid);
if (copyfunc == NULL)
- return emit_dummy_load__bcs(o);
-
- limit = intel_gen(devid) < 6 ? 500 : 5000;
+ return _emit_dummy_load__bcs(o, limit, timeout);
sb[0].bo = drm_intel_bo_alloc(bufmgr, "dummy_bo", 2048*2048*4, 4096);
igt_assert(sb[0].bo);
@@ -318,9 +361,25 @@ static void emit_dummy_load__rcs(struct test_output *o)
fb, 0, 0);
intel_batchbuffer_flush(batch);
+ if (timeout > 0)
+ ret = drm_intel_gem_bo_wait(fb->bo, timeout * NSEC_PER_SEC);
+
drm_intel_bo_unreference(sb[0].bo);
drm_intel_bo_unreference(sb[1].bo);
drm_intel_bo_unreference(sb[2].bo);
+
+ return ret;
+}
+
+static void emit_dummy_load__rcs(struct test_output *o, int seconds)
+{
+ static int ops_per_sec;
+
+ if (ops_per_sec == 0)
+ ops_per_sec = calibrate_dummy_load(o, "rcs",
+ _emit_dummy_load__rcs);
+
+ _emit_dummy_load__bcs(o, seconds * ops_per_sec, 0);
}
static void dpms_off_other_outputs(struct test_output *o)
@@ -820,10 +879,10 @@ static unsigned int run_test_step(struct test_output *o)
dpms_off_other_outputs(o);
if (o->flags & TEST_WITH_DUMMY_BCS)
- emit_dummy_load__bcs(o);
+ emit_dummy_load__bcs(o, 1);
if (o->flags & TEST_WITH_DUMMY_RCS)
- emit_dummy_load__rcs(o);
+
[Intel-gfx] [PATCH i-g-t 1/4] tests/kms_flip: Refactor blit code
From: Ville Syrjälä
Pull the code to emit a single blit to a separate function.
Signed-off-by: Ville Syrjälä
---
tests/kms_flip.c | 47 ---
1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index e579ce0..8b401be 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -162,6 +162,31 @@ static unsigned long gettime_us(void)
return ts.tv_sec * 100 + ts.tv_nsec / 1000;
}
+static void blit_copy(drm_intel_bo *dst, drm_intel_bo *src,
+ unsigned int width, unsigned int height,
+ unsigned int dst_pitch, unsigned int src_pitch)
+{
+ BLIT_COPY_BATCH_START(0);
+ OUT_BATCH((3 << 24) | /* 32 bits */
+ (0xcc << 16) | /* copy ROP */
+ dst_pitch);
+ OUT_BATCH(0 << 16 | 0);
+ OUT_BATCH(height << 16 | width);
+ OUT_RELOC_FENCED(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
0);
+ OUT_BATCH(0 << 16 | 0);
+ OUT_BATCH(src_pitch);
+ OUT_RELOC_FENCED(src, I915_GEM_DOMAIN_RENDER, 0, 0);
+ ADVANCE_BATCH();
+
+ if (batch->gen >= 6) {
+ BEGIN_BATCH(3, 0);
+ OUT_BATCH(XY_SETUP_CLIP_BLT_CMD);
+ OUT_BATCH(0);
+ OUT_BATCH(0);
+ ADVANCE_BATCH();
+ }
+}
+
static void emit_dummy_load__bcs(struct test_output *o)
{
int i, limit;
@@ -177,25 +202,9 @@ static void emit_dummy_load__bcs(struct test_output *o)
igt_assert(target_bo);
for (i = 0; i < limit; i++) {
- BLIT_COPY_BATCH_START(0);
- OUT_BATCH((3 << 24) | /* 32 bits */
- (0xcc << 16) | /* copy ROP */
- pitch);
- OUT_BATCH(0 << 16 | 0);
- OUT_BATCH(o->fb_height << 16 | o->fb_width);
- OUT_RELOC_FENCED(dummy_bo, I915_GEM_DOMAIN_RENDER,
I915_GEM_DOMAIN_RENDER, 0);
- OUT_BATCH(0 << 16 | 0);
- OUT_BATCH(pitch);
- OUT_RELOC_FENCED(target_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
- ADVANCE_BATCH();
-
- if (batch->gen >= 6) {
- BEGIN_BATCH(3, 0);
- OUT_BATCH(XY_SETUP_CLIP_BLT_CMD);
- OUT_BATCH(0);
- OUT_BATCH(0);
- ADVANCE_BATCH();
- }
+ blit_copy(dummy_bo, target_bo,
+ o->fb_width, o->fb_height,
+ pitch, pitch);
swap(dummy_bo, target_bo);
}
--
2.0.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/3] tests: Run lib/igt.cocci
From: Ville Syrjälä
Found some open coded min()/max()/swap() macros.
Signed-off-by: Ville Syrjälä
---
tests/drv_hangman.c | 4 ++--
tests/eviction_common.c | 5 +
tests/gem_seqno_wrap.c | 5 +
tests/gem_stress.c | 7 ++-
tests/kms_flip.c| 12 +++-
5 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index cdbded2..ec28c9d 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -33,6 +33,7 @@
#include "intel_chipset.h"
#include "drmtest.h"
+#include "igt_aux.h"
#include "igt_debugfs.h"
#include "ioctl_wrappers.h"
@@ -117,8 +118,7 @@ static void read_dfs(const char *fname, char *d, int maxlen)
static void _assert_dfs_entry(const char *fname, const char *s, bool inverse)
{
char tmp[1024];
- const int l = strlen(s) < sizeof(tmp) ?
- strlen(s) : sizeof(tmp);
+ const int l = min(strlen(s), sizeof(tmp));
read_dfs(fname, tmp, l + 1);
if (!inverse) {
diff --git a/tests/eviction_common.c b/tests/eviction_common.c
index 52578e2..4a12dcb 100644
--- a/tests/eviction_common.c
+++ b/tests/eviction_common.c
@@ -54,11 +54,8 @@ struct igt_eviction_test_ops
static void exchange_uint32_t(void *array, unsigned i, unsigned j)
{
uint32_t *i_arr = array;
- uint32_t i_tmp;
- i_tmp = i_arr[i];
- i_arr[i] = i_arr[j];
- i_arr[j] = i_tmp;
+ swap(i_arr[i], i_arr[j]);
}
static int minor_evictions(int fd, struct igt_eviction_test_ops *ops,
diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
index be4ab3d..7da7fdf 100644
--- a/tests/gem_seqno_wrap.c
+++ b/tests/gem_seqno_wrap.c
@@ -171,11 +171,8 @@ static void render_copyfunc(struct igt_buf *src,
static void exchange_uint(void *array, unsigned i, unsigned j)
{
unsigned *i_arr = array;
- unsigned i_tmp;
- i_tmp = i_arr[i];
- i_arr[i] = i_arr[j];
- i_arr[j] = i_tmp;
+ swap(i_arr[i], i_arr[j]);
}
static void run_sync_test(int num_buffers, bool verify)
diff --git a/tests/gem_stress.c b/tests/gem_stress.c
index 6e3a64c..9f20bde 100644
--- a/tests/gem_stress.c
+++ b/tests/gem_stress.c
@@ -570,11 +570,8 @@ static void init_set(unsigned set)
static void exchange_uint(void *array, unsigned i, unsigned j)
{
unsigned *i_arr = array;
- unsigned i_tmp;
- i_tmp = i_arr[i];
- i_arr[i] = i_arr[j];
- i_arr[j] = i_tmp;
+ swap(i_arr[i], i_arr[j]);
}
static void copy_tiles(unsigned *permutation)
@@ -741,7 +738,7 @@ static void init(void)
if (options.num_buffers == 0) {
tmp = gem_aperture_size(drm_fd);
- tmp = tmp > 256*(1024*1024) ? 256*(1024*1024) : tmp;
+ tmp = min(256 * (1024 * 1024), tmp);
num_buffers = 2 * tmp / options.scratch_buf_size / 3;
num_buffers /= 2;
igt_info("using %u buffers\n", num_buffers);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 041f46a..e579ce0 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -165,7 +165,7 @@ static unsigned long gettime_us(void)
static void emit_dummy_load__bcs(struct test_output *o)
{
int i, limit;
- drm_intel_bo *dummy_bo, *target_bo, *tmp_bo;
+ drm_intel_bo *dummy_bo, *target_bo;
struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
unsigned pitch = fb_info->stride;
@@ -197,9 +197,7 @@ static void emit_dummy_load__bcs(struct test_output *o)
ADVANCE_BATCH();
}
- tmp_bo = dummy_bo;
- dummy_bo = target_bo;
- target_bo = tmp_bo;
+ swap(dummy_bo, target_bo);
}
intel_batchbuffer_flush(batch);
@@ -282,16 +280,12 @@ static void emit_dummy_load__rcs(struct test_output *o)
dst = &sb[1];
for (i = 0; i < limit; i++) {
- struct igt_buf *tmp;
-
copyfunc(batch, NULL,
src, 0, 0,
o->fb_width, o->fb_height,
dst, 0, 0);
- tmp = src;
- src = dst;
- dst = tmp;
+ swap(src, dst);
}
intel_batchbuffer_flush(batch);
--
2.0.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3] lib/igt.cocci: Deal with min/max/swap
From: Ville Syrjälä Replace open coded min/max/swap with the macro invocation. Signed-off-by: Ville Syrjälä --- lib/igt.cocci | 36 1 file changed, 36 insertions(+) diff --git a/lib/igt.cocci b/lib/igt.cocci index adebb31..0d337bf 100644 --- a/lib/igt.cocci +++ b/lib/igt.cocci @@ -91,3 +91,39 @@ expression E; @@ - assert(E); + igt_assert(E); + +// Replace open-coded swap() +@@ +type T; +T a, b, tmp; +@@ +- tmp = a; +- a = b; +- b = tmp; ++ swap(a, b); + +// Replace open-coded min() +@@ +expression a; +expression b; +@@ +( +- ((a) < (b) ? (a) : (b)) ++ min(a, b) +| +- ((a) <= (b) ? (a) : (b)) ++ min(a, b) +) + +// Replace open-coded max() +@@ +expression a; +expression b; +@@ +( +- ((a) > (b) ? (a) : (b)) ++ max(a, b) +| +- ((a) >= (b) ? (a) : (b)) ++ max(a, b) +) -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/3] lib: Add swap() macro
From: Ville Syrjälä
swap() will swap its two arguments while keeping the required
tmp variable hidden. Makes for neater code.
Signed-off-by: Ville Syrjälä
---
lib/igt_aux.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 6c83c53..63e1b06 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size,
unsigned mode);
#define min(a, b) ((a) < (b) ? (a) : (b))
#define max(a, b) ((a) > (b) ? (a) : (b))
+#define swap(a, b) do {\
+ typeof(a) _tmp = (a); \
+ (a) = (b); \
+ (b) = _tmp; \
+} while (0)
+
#endif /* IGT_AUX_H */
--
2.0.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Updated drm-intel-testing
Hi all, New -testing cycle with cool stuff, first round for 3.20 - hah no typo this time around in the version ;-) - dual-dsi enabling from Gaurav with prep work from Jani - reshuffling the ring init code to move towards a clean sw/hw state setup split - ring free space refactoring from Dave Gordon - s/seqno/request/ rework from John Harrison - psr support for vlv/chv from Rodrigo - skl mmio flip support from Damien - and the usual bits&pieces all over Happy testing! Cheers, 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 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > Currently we initialise the rings, add the first context switch to the
> > > ring and execute our golden state then enable (aliasing or full) ppgtt.
> > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > MI_LRI, we end up executing the context switch and golden render state
> > > with an invalid PD generating page faults. To solve this issue, first do
> > > the ppgtt PD setup, then set the default context and write the commands
> > > to run the render state into the ring, before we activate the ring. This
> > > allows us to be sure that the register state is valid before we begin
> > > execution.
> > >
> > > This was spotted when writing the seqno/request conversion, but only with
> > > the ERROR capture did I realise that it was a necessity now.
> > >
> > > RFC: cleanup the error handling in i915_gem_init_hw.
> > >
> > > Signed-off-by: Chris Wilson
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index c1c11418231b..c13842d3cbc9 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > >*/
> > > init_unused_rings(dev);
> > >
> > > - for_each_ring(ring, dev_priv, i) {
> > > - ret = ring->init_hw(ring);
> > > - if (ret)
> > > - return ret;
> > > - }
> > > -
> > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> >
> > This is going to assume ring->head/tail are already valid?
>
> We write into the ring obj, not the ring itself, which should be setup
> during the various intel_init_engine, i.e. the backing storage is
> independent of the actual registers.
But there's still intel_ring_advance which calls ->write_tail all over the
place. So we drop all these mmio writes into nirvana since we'll reset the
ring later on?
-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 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 02:38:46PM +, Chris Wilson wrote:
> On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > > Currently we initialise the rings, add the first context switch to the
> > > ring and execute our golden state then enable (aliasing or full) ppgtt.
> > > However, as we enable ppgtt using direct MMIO but load the PD using
> > > MI_LRI, we end up executing the context switch and golden render state
> > > with an invalid PD generating page faults. To solve this issue, first do
> > > the ppgtt PD setup, then set the default context and write the commands
> > > to run the render state into the ring, before we activate the ring. This
> > > allows us to be sure that the register state is valid before we begin
> > > execution.
> > >
> > > This was spotted when writing the seqno/request conversion, but only with
> > > the ERROR capture did I realise that it was a necessity now.
> > >
> > > RFC: cleanup the error handling in i915_gem_init_hw.
> > >
> > > Signed-off-by: Chris Wilson
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > > 2 files changed, 16 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index c1c11418231b..c13842d3cbc9 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > >*/
> > > init_unused_rings(dev);
> > >
> > > - for_each_ring(ring, dev_priv, i) {
> > > - ret = ring->init_hw(ring);
> > > - if (ret)
> > > - return ret;
> > > - }
> > > -
> > > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
> >
> > This is going to assume ring->head/tail are already valid?
>
> We write into the ring obj, not the ring itself, which should be setup
> during the various intel_init_engine, i.e. the backing storage is
> independent of the actual registers.
I mean the software shadows, not the registers themselves. When the GPU
hangs I expect rign->head != ring->tail. So what makes those two identical
again after the GPU reset?
>
> If you take it to the logical conclusion, each operation on the ring is
> just a transaction that is buffered until it is commited to the
> hardware, or rolled back in its entirety if aborted. We can then queue
> up multiple transactions before first enabling the hardware, or equally
> preserve such transactions across a GPU reset.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/drm_read: Abuse read(drm)
On Fri, Dec 05, 2014 at 01:14:40PM +, Chris Wilson wrote:
> +static void assert_empty(int fd)
> +{
> + struct pollfd pfd = {fd, POLLIN };
> + igt_assert(poll(&pfd, 1, 0) == 0);
> +}
> +
> +static void generate_event(int fd)
> +{
> + union drm_wait_vblank vbl;
> +
> + /* We assume that pipe 0 is running */
This assumption falls short on gen2/3 with lvds only, and might fall short
if a previous test wreaked havoc. Also dpms tends to foul up kms tests,
too. I think we do need the full-blown modeset dance here unfortunately.
-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/bdw: Add WaHdcDisableFetchWhenMasked
On Thu, Dec 04, 2014 at 05:25:56PM +0200, Ville Syrjälä wrote: > On Thu, Dec 04, 2014 at 03:07:52PM +, Michel Thierry wrote: > > We already have it for chv, but was missing for bdw. > > > > Signed-off-by: Michel Thierry > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 788e1b6..91ddcd1 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -756,9 +756,11 @@ static int bdw_init_workarounds(struct intel_engine_cs > > *ring) > > * workaround for for a possible hang in the unlikely event a TLB > > * invalidation occurs during a PSD flush. > > */ > > + /* WaHdcDisableFetchWhenMasked:bdw */ > > /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */ > > WA_SET_BIT_MASKED(HDC_CHICKEN0, > > HDC_FORCE_NON_COHERENT | > > + HDC_DONOT_FETCH_MEM_WHEN_MASKED | > > (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0)); > > 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 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 04:31:35PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> > Currently we initialise the rings, add the first context switch to the
> > ring and execute our golden state then enable (aliasing or full) ppgtt.
> > However, as we enable ppgtt using direct MMIO but load the PD using
> > MI_LRI, we end up executing the context switch and golden render state
> > with an invalid PD generating page faults. To solve this issue, first do
> > the ppgtt PD setup, then set the default context and write the commands
> > to run the render state into the ring, before we activate the ring. This
> > allows us to be sure that the register state is valid before we begin
> > execution.
> >
> > This was spotted when writing the seqno/request conversion, but only with
> > the ERROR capture did I realise that it was a necessity now.
> >
> > RFC: cleanup the error handling in i915_gem_init_hw.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> > 2 files changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index c1c11418231b..c13842d3cbc9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
> > */
> > init_unused_rings(dev);
> >
> > - for_each_ring(ring, dev_priv, i) {
> > - ret = ring->init_hw(ring);
> > - if (ret)
> > - return ret;
> > - }
> > -
> > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > i915_gem_l3_remap(&dev_priv->ring[RCS], i);
>
> This is going to assume ring->head/tail are already valid?
We write into the ring obj, not the ring itself, which should be setup
during the various intel_init_engine, i.e. the backing storage is
independent of the actual registers.
If you take it to the logical conclusion, each operation on the ring is
just a transaction that is buffered until it is commited to the
hardware, or rolled back in its entirety if aborted. We can then queue
up multiple transactions before first enabling the hardware, or equally
preserve such transactions across a GPU reset.
-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 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller
On Fri, Dec 05, 2014 at 06:20:43PM +0530, Singh, Gaurav K wrote: > > On 12/4/2014 4:52 PM, Daniel Vetter wrote: > >On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote: > >>On Thu, 04 Dec 2014, Gaurav K Singh wrote: > >>>This patch is in preparation of DSI dual link panels. For dual link > >>>panels, few packets needs to be sent to Port A or Port C or both. Based > >>>on the portno from MIPI Sequence Block#53, these sequences needs to be > >>>sent accordingly. > >>> > >>>v2: Addressed review comments by Jani > >>> - port variables named properly > >>> > >>>Signed-off-by: Gaurav K Singh > >>Reviewed-by: Jani Nikula > >Merged the first two patches, thanks. > >-Daniel > Hi Daniel, > > Thanks. I have addressed Jani's review comments too for the rest of the > patches. Well looking through the resend patches your changes look a bit minimal. Which might be the right thing to do, but review is also about communication and sharing expert knowledge. So please follow up to the questions from Jani where you didn't adjust the code. And if there's any follow-up patches from those discussions please submit them. Another part: Your editor doesn't seem to align continuation lines quite how we usually do that. checkpatch --strict will catch that. Two big rules: - function paramater lists should be aligned to the opening ( on the next line. - Continuations of boolean logic checks (e.g. in if checks) same, with the special rule that && or || should be on the previous line as the last thing. - continuation of any other long lines should be indented 1 or two spaces (just to make sure it sticks out without looking jarring). Especially the first two help readability a lot of you're used to them, so please do a follow-up patch to rectify this in intel_dsi*.c. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
On Fri, Dec 05, 2014 at 02:15:22PM +, Chris Wilson wrote:
> Currently we initialise the rings, add the first context switch to the
> ring and execute our golden state then enable (aliasing or full) ppgtt.
> However, as we enable ppgtt using direct MMIO but load the PD using
> MI_LRI, we end up executing the context switch and golden render state
> with an invalid PD generating page faults. To solve this issue, first do
> the ppgtt PD setup, then set the default context and write the commands
> to run the render state into the ring, before we activate the ring. This
> allows us to be sure that the register state is valid before we begin
> execution.
>
> This was spotted when writing the seqno/request conversion, but only with
> the ERROR capture did I realise that it was a necessity now.
>
> RFC: cleanup the error handling in i915_gem_init_hw.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_gem.c | 20 ++--
> drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c1c11418231b..c13842d3cbc9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
>*/
> init_unused_rings(dev);
>
> - for_each_ring(ring, dev_priv, i) {
> - ret = ring->init_hw(ring);
> - if (ret)
> - return ret;
> - }
> -
> for (i = 0; i < NUM_L3_SLICES(dev); i++)
> i915_gem_l3_remap(&dev_priv->ring[RCS], i);
This is going to assume ring->head/tail are already valid?
>
> + ret = i915_ppgtt_init_hw(dev);
> + if (ret && ret != -EIO) {
> + DRM_ERROR("PPGTT enable failed %d\n", ret);
> + i915_gem_cleanup_ringbuffer(dev);
> + }
> +
> /*
>* XXX: Contexts should only be initialized once. Doing a switch to the
>* default context switch however is something we'd like to do after
> @@ -4820,10 +4820,10 @@ i915_gem_init_hw(struct drm_device *dev)
> return ret;
> }
>
> - ret = i915_ppgtt_init_hw(dev);
> - if (ret && ret != -EIO) {
> - DRM_ERROR("PPGTT enable failed %d\n", ret);
> - i915_gem_cleanup_ringbuffer(dev);
> + for_each_ring(ring, dev_priv, i) {
> + ret = ring->init_hw(ring);
> + if (ret)
> + return ret;
> }
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index de38b04135d2..95ebce73d46d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -585,6 +585,9 @@ static int init_ring_common(struct intel_engine_cs *ring)
> I915_WRITE_HEAD(ring, 0);
> (void)I915_READ_HEAD(ring);
>
> + I915_WRITE_HEAD(ring, ringbuf->head);
> + I915_WRITE_TAIL(ring, ringbuf->head);
Here too.
So what happens on GPU reset?
> +
> I915_WRITE_CTL(ring,
> ((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES)
> | RING_VALID);
> @@ -592,7 +595,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
> /* 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) == ringbuf->head, 50)) {
> DRM_ERROR("%s initialization failed "
> "ctl %08x (valid? %d) head %08x tail %08x start %08x
> [expected %08lx]\n",
> ring->name,
> @@ -603,9 +606,9 @@ static int init_ring_common(struct intel_engine_cs *ring)
> goto out;
> }
>
> + I915_WRITE_TAIL(ring, ringbuf->tail);
> +
> ringbuf->last_retired_head = -1;
> - ringbuf->head = I915_READ_HEAD(ring);
> - ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR;
> intel_ring_update_space(ringbuf);
>
> memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
> --
> 2.1.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] tests/pm_rps: vlv: round middle point to freq supported by HW
When setting the calculated middle frequency value the test assumes that
the HW/kernel rounds this value according to a 50MHz step value. This is
not so at least on VLV/CHV, on my B0 BYT-M for example this step value
is 22MHz, so there the test will fail.
To fix this get the nearest supported value by setting the target
frequency as a min or max frequency and read it back. The kernel will
round the returned value to the nearest supported.
v2:
- remove the 50MHz rounding that was done for non-VLV platforms, the new
way of rounding should provide the correct value for all platforms
(Ville)
Signed-off-by: Imre Deak
---
tests/pm_rps.c | 44 +---
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index fa6c014..180a147 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -120,7 +120,7 @@ static void wait_freq_settle(void)
}
}
-static int do_writeval(FILE *filp, int val, int lerrno)
+static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
{
int ret, orig;
@@ -131,18 +131,21 @@ static int do_writeval(FILE *filp, int val, int lerrno)
if (lerrno) {
/* Expecting specific error */
igt_assert(ret == EOF && errno == lerrno);
- igt_assert(readval(filp) == orig);
+ if (readback_check)
+ igt_assert_eq(readval(filp), orig);
} else {
/* Expecting no error */
igt_assert_neq(ret, 0);
wait_freq_settle();
- igt_assert(readval(filp) == val);
+ if (readback_check)
+ igt_assert_eq(readval(filp), val);
}
return ret;
}
-#define writeval(filp, val) do_writeval(filp, val, 0)
-#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
+#define writeval(filp, val) do_writeval(filp, val, 0, true)
+#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
+#define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
static void checkit(const int *freqs)
{
@@ -342,12 +345,39 @@ static void do_load_gpu(void)
load_helper_stop();
}
+/* Return a frequency rounded by HW to the nearest supported value */
+static int get_hw_rounded_freq(int target)
+{
+ int freqs[NUMFREQ];
+ int old_freq;
+ int idx;
+ int ret;
+
+ read_freqs(freqs);
+
+ if (freqs[MIN] > target)
+ idx = MIN;
+ else
+ idx = MAX;
+
+ old_freq = freqs[idx];
+ writeval_nocheck(stuff[idx].filp, target);
+ read_freqs(freqs);
+ ret = freqs[idx];
+ writeval_nocheck(stuff[idx].filp, old_freq);
+
+ return ret;
+}
+
static void min_max_config(void (*check)(void), bool load_gpu)
{
int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
- /* hw (and so kernel) currently rounds to 50 MHz ... */
- fmid = fmid / 50 * 50;
+ /*
+* hw (and so kernel) rounds to the nearest value supported by
+* the given platform.
+*/
+ fmid = get_hw_rounded_freq(fmid);
igt_debug("\nCheck original min and max...\n");
if (load_gpu)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Enable HDMI Clock recovery for Pipe C
On Fri, Dec 05, 2014 at 03:23:59PM +0200, Imre Deak wrote: > Hm, right I should have checked that first. And it could be also the > reason why AZX_DCAPS_I915_POWERWELL was left out for everything but > HSW/BDW. I will send a patch to fix that in i915. Not terribly in favour of perpetuing that hack. It's proven sufficient trouble imo already. -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: Infrastructure for supporting different GGTT views per object
On Fri, Dec 05, 2014 at 12:11:45PM +, Tvrtko Ursulin wrote:
> +struct i915_ggtt_view {
> + enum i915_ggtt_view_type type;
> +
> + struct sg_table *pages;
> +};
Minor nit on semantics, not really worth a resend (except when you need
one anyway because of the detailed review): Imo the sg_table should be in
the vma directly, not in the gtt_view. Otherwise we can't just do a memcmp
if the view paramaters grow more fancy than just the type key. And maybe
call it sg_table, not pages, to avoid confusion with the real obj->pages
backing storage pointer.
Anyway just bikesheds ;-)
-Daniel
> +
> +extern const struct i915_ggtt_view i915_ggtt_view_normal;
> +
> enum i915_cache_level;
> +
> /**
> * A VMA represents a GEM BO that is bound into an address space. Therefore,
> a
> * VMA's presence cannot be guaranteed before binding, or after unbinding the
> @@ -129,6 +142,15 @@ struct i915_vma {
> #define PTE_READ_ONLY(1<<2)
> unsigned int bound : 4;
>
> + /**
> + * Support different GGTT views into the same object.
> + * This means there can be multiple VMA mappings per object and per VM.
> + * i915_ggtt_view_type is used to distinguish between those entries.
> + * The default one of zero (I915_GGTT_VIEW_NORMAL) is default and also
> + * assumed in GEM functions which take no ggtt view parameter.
> + */
> + struct i915_ggtt_view ggtt_view;
> +
> /** This object's place on the active/inactive lists */
> struct list_head mm_list;
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c4536e1..f97479a 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -718,10 +718,8 @@ static u32 capture_pinned_bo(struct
> drm_i915_error_buffer *err,
> break;
>
> list_for_each_entry(vma, &obj->vma_list, vma_link)
> - if (vma->vm == vm && vma->pin_count > 0) {
> + if (vma->vm == vm && vma->pin_count > 0)
> capture_bo(err++, vma);
> - break;
> - }
> }
>
> return err - first;
> @@ -1096,10 +1094,8 @@ static void i915_gem_capture_vm(struct
> drm_i915_private *dev_priv,
>
> list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> list_for_each_entry(vma, &obj->vma_list, vma_link)
> - if (vma->vm == vm && vma->pin_count > 0) {
> + if (vma->vm == vm && vma->pin_count > 0)
> i++;
> - break;
> - }
> }
> error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx];
>
> --
> 2.1.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
[Intel-gfx] [PATCH 1/2] drm/i915: Detect page faults during hangcheck
On Sandybridge+, the GPU provides the ERROR register for detecting page
faults. Hook this up to our hangcheck so that we can dump the error
state soon after such an event occurs. This would be better inside an
interrupt handler, but it serves a purpose here as it detects that our
initial context setup is invalid...
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_irq.c | 5 +
drivers/gpu/drm/i915/intel_uncore.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7913a72ce30a..eb2149b941e4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2969,6 +2969,11 @@ static void i915_hangcheck_elapsed(unsigned long data)
if (!i915.enable_hangcheck)
return;
+ if (INTEL_INFO(dev_priv)->gen >= 6 && I915_READ(ERROR_GEN6)) {
+ i915_handle_error(dev, false, "GPU reported a page fault");
+ I915_WRITE(ERROR_GEN6, 0);
+ }
+
for_each_ring(ring, dev_priv, i) {
u64 acthd;
u32 seqno;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 0655b44651a7..67f2e24c5bc5 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -535,6 +535,8 @@ static void __intel_uncore_early_sanitize(struct drm_device
*dev,
if (IS_GEN6(dev) || IS_GEN7(dev))
__raw_i915_write32(dev_priv, GTFIFODBG,
__raw_i915_read32(dev_priv, GTFIFODBG));
+ if (INTEL_INFO(dev)->gen >= 6)
+ __raw_i915_write32(dev_priv, ERROR_GEN6, 0);
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Reorder hw init to avoid executing with invalid context/mm state
Currently we initialise the rings, add the first context switch to the
ring and execute our golden state then enable (aliasing or full) ppgtt.
However, as we enable ppgtt using direct MMIO but load the PD using
MI_LRI, we end up executing the context switch and golden render state
with an invalid PD generating page faults. To solve this issue, first do
the ppgtt PD setup, then set the default context and write the commands
to run the render state into the ring, before we activate the ring. This
allows us to be sure that the register state is valid before we begin
execution.
This was spotted when writing the seqno/request conversion, but only with
the ERROR capture did I realise that it was a necessity now.
RFC: cleanup the error handling in i915_gem_init_hw.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c | 20 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++---
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c1c11418231b..c13842d3cbc9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4796,15 +4796,15 @@ i915_gem_init_hw(struct drm_device *dev)
*/
init_unused_rings(dev);
- for_each_ring(ring, dev_priv, i) {
- ret = ring->init_hw(ring);
- if (ret)
- return ret;
- }
-
for (i = 0; i < NUM_L3_SLICES(dev); i++)
i915_gem_l3_remap(&dev_priv->ring[RCS], i);
+ ret = i915_ppgtt_init_hw(dev);
+ if (ret && ret != -EIO) {
+ DRM_ERROR("PPGTT enable failed %d\n", ret);
+ i915_gem_cleanup_ringbuffer(dev);
+ }
+
/*
* XXX: Contexts should only be initialized once. Doing a switch to the
* default context switch however is something we'd like to do after
@@ -4820,10 +4820,10 @@ i915_gem_init_hw(struct drm_device *dev)
return ret;
}
- ret = i915_ppgtt_init_hw(dev);
- if (ret && ret != -EIO) {
- DRM_ERROR("PPGTT enable failed %d\n", ret);
- i915_gem_cleanup_ringbuffer(dev);
+ for_each_ring(ring, dev_priv, i) {
+ ret = ring->init_hw(ring);
+ if (ret)
+ return ret;
}
return ret;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index de38b04135d2..95ebce73d46d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -585,6 +585,9 @@ static int init_ring_common(struct intel_engine_cs *ring)
I915_WRITE_HEAD(ring, 0);
(void)I915_READ_HEAD(ring);
+ I915_WRITE_HEAD(ring, ringbuf->head);
+ I915_WRITE_TAIL(ring, ringbuf->head);
+
I915_WRITE_CTL(ring,
((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES)
| RING_VALID);
@@ -592,7 +595,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
/* 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) == ringbuf->head, 50)) {
DRM_ERROR("%s initialization failed "
"ctl %08x (valid? %d) head %08x tail %08x start %08x
[expected %08lx]\n",
ring->name,
@@ -603,9 +606,9 @@ static int init_ring_common(struct intel_engine_cs *ring)
goto out;
}
+ I915_WRITE_TAIL(ring, ringbuf->tail);
+
ringbuf->last_retired_head = -1;
- ringbuf->head = I915_READ_HEAD(ring);
- ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR;
intel_ring_update_space(ringbuf);
memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: release struct_mutex on the i915_gem_init_hw fail path
On Fri, Dec 05, 2014 at 05:03:14AM -0800, Jeremiah Mahler wrote:
> Jani,
>
> On Fri, Dec 05, 2014 at 02:17:42PM +0200, Jani Nikula wrote:
> > Release struct_mutex if init_rings() fails.
> >
> > This is a regression introduced in
> > commit 35a57ffbb10840af219eeaf64718434242bb7c76
> > Author: Daniel Vetter
> > Date: Thu Nov 20 00:33:07 2014 +0100
> >
> > drm/i915: Only init engines once
> >
> > Reported-by: Wei Yongjun
> > Signed-off-by: Jani Nikula
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 16 +++-
> > 1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index c1c11418231b..e3ce4bef22a3 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4860,22 +4860,18 @@ int i915_gem_init(struct drm_device *dev)
> > }
> >
> > ret = i915_gem_init_userptr(dev);
> > - if (ret) {
> > - mutex_unlock(&dev->struct_mutex);
> > - return ret;
> > - }
> > + if (ret)
> > + goto out_unlock;
> >
> > i915_gem_init_global_gtt(dev);
> >
> > ret = i915_gem_context_init(dev);
> > - if (ret) {
> > - mutex_unlock(&dev->struct_mutex);
> > - return ret;
> > - }
> > + if (ret)
> > + goto out_unlock;
> >
> > ret = dev_priv->gt.init_rings(dev);
> > if (ret)
> > - return ret;
> > + goto out_unlock;
> >
> > ret = i915_gem_init_hw(dev);
> > if (ret == -EIO) {
> > @@ -4887,6 +4883,8 @@ int i915_gem_init(struct drm_device *dev)
> > atomic_set_mask(I915_WEDGED,
> > &dev_priv->gpu_error.reset_counter);
> > ret = 0;
> > }
> > +
> > +out_unlock:
> > mutex_unlock(&dev->struct_mutex);
> >
> > return ret;
> > --
> > 2.1.3
> >
>
> Yes, this looks much better.
Oh well confusion in my mailbox. I've dropped Wei's patch and merged this
one instead.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: allow requesting the audio power well for all platforms
On Fri, 2014-12-05 at 15:28 +0200, Imre Deak wrote:
> So far we only allowed HSW and BDW to request for the audio power
> domain, but it is also needed at least on VLV/CHV. There is no need
> for this restriction, since the power domain->power well mapping should
> take care of the distinctions between platforms.
>
> Spotted-by: Jani Nikula
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8a2bd18..58204ab 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -50,7 +50,7 @@
> * present for a given platform.
> */
>
> -static struct i915_power_domains *hsw_pwr;
> +static struct i915_power_domains *i915_pwr;
>
> #define for_each_power_well(i, power_well, domain_mask, power_domains)
> \
> for (i = 0; \
> @@ -1098,10 +1098,8 @@ int intel_power_domains_init(struct drm_i915_private
> *dev_priv)
>*/
> if (IS_HASWELL(dev_priv->dev)) {
> set_power_wells(power_domains, hsw_power_wells);
> - hsw_pwr = power_domains;
> } else if (IS_BROADWELL(dev_priv->dev)) {
> set_power_wells(power_domains, bdw_power_wells);
> - hsw_pwr = power_domains;
> } else if (IS_CHERRYVIEW(dev_priv->dev)) {
> set_power_wells(power_domains, chv_power_wells);
> } else if (IS_VALLEYVIEW(dev_priv->dev)) {
> @@ -1110,6 +1108,8 @@ int intel_power_domains_init(struct drm_i915_private
> *dev_priv)
> set_power_wells(power_domains, i9xx_always_on_power_well);
> }
>
> + i915_pwr = power_domains;
> +
> return 0;
> }
>
> @@ -1146,7 +1146,7 @@ void intel_power_domains_fini(struct drm_i915_private
> *dev_priv)
>* we're going to unload/reload. */
> intel_display_set_init_power(dev_priv, true);
>
> - hsw_pwr = NULL;
> + i915_pwr = NULL;
> }
>
> static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
> @@ -1360,10 +1360,10 @@ int i915_request_power_well(void)
> {
> struct drm_i915_private *dev_priv;
>
> - if (!hsw_pwr)
> + if (!i915_pwr)
> return -ENODEV;
>
> - dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> + dev_priv = container_of(i915_pwr, struct drm_i915_private,
> power_domains);
> intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> return 0;
> @@ -1375,10 +1375,10 @@ int i915_release_power_well(void)
> {
> struct drm_i915_private *dev_priv;
>
> - if (!hsw_pwr)
> + if (!i915_pwr)
> return -ENODEV;
>
> - dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> + dev_priv = container_of(i915_pwr, struct drm_i915_private,
> power_domains);
> intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> return 0;
> @@ -1395,10 +1395,10 @@ int i915_get_cdclk_freq(void)
> {
> struct drm_i915_private *dev_priv;
>
> - if (!hsw_pwr)
> + if (!i915_pwr)
> return -ENODEV;
Err, we should also WARN and return here for !HAS_DDI. This is used for
restoring the audio BCLK M/N values in the HSW/BDW extended mode
registers, but there is no corresponding registers on other platforms.
>
> - dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> + dev_priv = container_of(i915_pwr, struct drm_i915_private,
> power_domains);
>
> return intel_ddi_get_cdclk_freq(dev_priv);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [ANNOUNCE][RFC] KVMGT - the implementation of Intel GVT-g(full GPU virtualization) for KVM
On Fri, Dec 05, 2014 at 09:50:21AM +0100, Gerd Hoffmann wrote: > > > > https://www.usenix.org/conference/atc14/technical-sessions/presentation/tian > > /me goes read this. > > A few comments on the kernel stuff (brief look so far, also > compile-tested only, intel gfx on my test machine is too old). > > * Noticed the kernel bits don't even compile when configured as >module. Everything (vgt, i915, kvm) must be compiled into the >kernel. > * Design approach still seems to be i915 on vgt not the other way >around. Yeah done a quick read-through of just the i915 bits too, same comment. I guess this is just the first RFC and the redesign we've discussed about already with xengt is in progress somewhere? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 4/4] drm/i915: Additional request structure tracing
From: John Harrison
Added the request structure's 'uniq' identifier to the trace information. Also
renamed the '_complete' trace event to '_notify' as it actually happens in the
IRQ 'notify_ring()' function. The intention is to add a new '_complete' trace
event which occurs when a request structure is actually marked as complete.
However, at the moment the completion status is re-tested every time the query
is made so there isn't a completion event as such.
v2: New patch added to series.
v3: Rebased to remove completion caching as that is apparently contentious.
Change-Id: Ic9bcde67d175c6c03b96217cdcb6e4cc4aa45d67
For: VIZ-4377
Signed-off-by: John Harrison
Reviewed-by: Thomas Daniel
---
drivers/gpu/drm/i915/i915_irq.c |2 +-
drivers/gpu/drm/i915/i915_trace.h | 22 --
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7913a72..08a5a4b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1013,7 +1013,7 @@ static void notify_ring(struct drm_device *dev,
if (!intel_ring_initialized(ring))
return;
- trace_i915_gem_request_complete(ring);
+ trace_i915_gem_request_notify(ring);
wake_up_all(&ring->irq_queue);
}
diff --git a/drivers/gpu/drm/i915/i915_trace.h
b/drivers/gpu/drm/i915/i915_trace.h
index 2ade958..6058a01 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -406,6 +406,7 @@ DECLARE_EVENT_CLASS(i915_gem_request,
TP_STRUCT__entry(
__field(u32, dev)
__field(u32, ring)
+__field(u32, uniq)
__field(u32, seqno)
),
@@ -414,11 +415,13 @@ DECLARE_EVENT_CLASS(i915_gem_request,
i915_gem_request_get_ring(req);
__entry->dev = ring->dev->primary->index;
__entry->ring = ring->id;
+ __entry->uniq = req ? req->uniq : 0;
__entry->seqno = i915_gem_request_get_seqno(req);
),
- TP_printk("dev=%u, ring=%u, seqno=%u",
- __entry->dev, __entry->ring, __entry->seqno)
+ TP_printk("dev=%u, ring=%u, uniq=%u, seqno=%u",
+ __entry->dev, __entry->ring, __entry->uniq,
+ __entry->seqno)
);
DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
@@ -426,7 +429,7 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
TP_ARGS(req)
);
-TRACE_EVENT(i915_gem_request_complete,
+TRACE_EVENT(i915_gem_request_notify,
TP_PROTO(struct intel_engine_cs *ring),
TP_ARGS(ring),
@@ -451,6 +454,11 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_retire,
TP_ARGS(req)
);
+DEFINE_EVENT(i915_gem_request, i915_gem_request_complete,
+ TP_PROTO(struct drm_i915_gem_request *req),
+ TP_ARGS(req)
+);
+
TRACE_EVENT(i915_gem_request_wait_begin,
TP_PROTO(struct drm_i915_gem_request *req),
TP_ARGS(req),
@@ -458,6 +466,7 @@ TRACE_EVENT(i915_gem_request_wait_begin,
TP_STRUCT__entry(
__field(u32, dev)
__field(u32, ring)
+__field(u32, uniq)
__field(u32, seqno)
__field(bool, blocking)
),
@@ -473,14 +482,15 @@ TRACE_EVENT(i915_gem_request_wait_begin,
i915_gem_request_get_ring(req);
__entry->dev = ring->dev->primary->index;
__entry->ring = ring->id;
+ __entry->uniq = req ? req->uniq : 0;
__entry->seqno = i915_gem_request_get_seqno(req);
__entry->blocking =
mutex_is_locked(&ring->dev->struct_mutex);
),
- TP_printk("dev=%u, ring=%u, seqno=%u, blocking=%s",
- __entry->dev, __entry->ring, __entry->seqno,
- __entry->blocking ? "yes (NB)" : "no")
+ TP_printk("dev=%u, ring=%u, uniq=%u, seqno=%u, blocking=%s",
+ __entry->dev, __entry->ring, __entry->uniq,
+ __entry->seqno, __entry->blocking ? "yes (NB)" : "no")
);
DEFINE_EVENT(i915_gem_request, i915_gem_request_wait_end,
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/4] drm/i915: Add unique id to the request structure for debugging
From: John Harrison
For debugging purposes, it is useful to be able to uniquely identify a given
request structure as it works its way through the system. This becomes
especially tricky once the seqno value is lazily allocated as then the request
has nothing but its pointer to identify it for much of its life.
Change-Id: Ie76b2268b940467f4cdf5a4ba6f5a54cbb96445d
For: VIZ-4377
Signed-off-by: John Harrison
Reviewed-by: Thomas Daniel
---
drivers/gpu/drm/i915/i915_drv.h |4
drivers/gpu/drm/i915/intel_lrc.c|2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c |2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f..18dfa1e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1787,6 +1787,8 @@ struct drm_i915_private {
void (*stop_ring)(struct intel_engine_cs *ring);
} gt;
+ uint32_t request_uniq;
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
@@ -2019,6 +2021,8 @@ struct drm_i915_gem_request {
struct drm_i915_file_private *file_priv;
/** file_priv list entry for this request */
struct list_head client_list;
+
+ uint32_t uniq;
};
void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 35ed2f1..1882e11 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -880,6 +880,7 @@ static int logical_ring_alloc_request(struct
intel_engine_cs *ring,
struct intel_context *ctx)
{
struct drm_i915_gem_request *request;
+ struct drm_i915_private *dev_private = ring->dev->dev_private;
int ret;
if (ring->outstanding_lazy_request)
@@ -899,6 +900,7 @@ static int logical_ring_alloc_request(struct
intel_engine_cs *ring,
kref_init(&request->ref);
request->ring = ring;
+ request->uniq = dev_private->request_uniq++;
ret = i915_gem_get_seqno(ring->dev, &request->seqno);
if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2c6c6f8..d496c97 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2026,6 +2026,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
{
int ret;
struct drm_i915_gem_request *request;
+ struct drm_i915_private *dev_private = ring->dev->dev_private;
if (ring->outstanding_lazy_request)
return 0;
@@ -2036,6 +2037,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
kref_init(&request->ref);
request->ring = ring;
+ request->uniq = dev_private->request_uniq++;
ret = i915_gem_get_seqno(ring->dev, &request->seqno);
if (ret) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 2/4] drm/i915: Zero fill the request structure
From: John Harrison There is a general theory that kzmalloc is better/safer than kmalloc, especially for interesting data structures. This change updates the request structure allocation to be zero filled. Change-Id: I68715ef758025fab8db763941ef63bf60d7031e2 For: VIZ-4377 Signed-off-by: John Harrison Reviewed-by: Thomas Daniel --- drivers/gpu/drm/i915/intel_lrc.c|2 +- drivers/gpu/drm/i915/intel_ringbuffer.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 52e9952..35ed2f1 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -885,7 +885,7 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring, if (ring->outstanding_lazy_request) return 0; - request = kmalloc(sizeof(*request), GFP_KERNEL); + request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 79b4ca5..2c6c6f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2030,7 +2030,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring) if (ring->outstanding_lazy_request) return 0; - request = kmalloc(sizeof(*request), GFP_KERNEL); + request = kzalloc(sizeof(*request), GFP_KERNEL); if (request == NULL) return -ENOMEM; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 0/4] Replace seqno values with request structures
From: John Harrison There is a general feeling that it is better to move away from using a simple integer 'seqno' value to track batch buffer completion. Instead, the request structure should be used. That provides for much more flexibility going forwards. Especially which things like a GPU scheduler (which can re-order batch buffers and hence seqnos after submission to the hardware), Android sync points and other such features which potentially make seqno usage more and more complex. This patch set does the work of converting most of the driver to use request structures in preference to seqno values. The only place left that still uses seqnos is the semaphore code. It was decided to leave that alone for the time being as the semaphores are hardware based and the hardware only understands seqno values. v2: Rebased to newer nightly tree which significantly changed some of the display MMIO flip code (including making __wait_request public and renaming it). v3: Rebased to yet another nightly tree which included execlist pin/unpin patches. NB: this is based on a tree including a pending update for an unpin bug. Also includes a fix for a clash with the shrinker. v4: Remaining patches rebased to remove caching of completion status patches. This is now future work to be done in a different manner. [Patches against drm-intel-nightly tree fetched 03/12/2014] John Harrison (4): drm/i915: Fix up seqno -> request merge issues drm/i915: Zero fill the request structure drm/i915: Add unique id to the request structure for debugging drm/i915: Additional request structure tracing drivers/gpu/drm/i915/i915_drv.h |4 drivers/gpu/drm/i915/i915_irq.c |2 +- drivers/gpu/drm/i915/i915_trace.h | 22 -- drivers/gpu/drm/i915/intel_display.c|6 ++ drivers/gpu/drm/i915/intel_lrc.c|4 +++- drivers/gpu/drm/i915/intel_ringbuffer.c |4 +++- 6 files changed, 29 insertions(+), 13 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/4] drm/i915: Fix up seqno -> request merge issues
From: John Harrison
The display related patches earlier in this series were edited during merge to
improve the request unreferencing. Specifically, the need for de-referencing at
interrupt time was removed. However, the resulting code did a 'deref(req) ; req
= NULL' sequence rather than using the 'req_assign(req, NULL)' wrapper. The two
are functionally equivalent, but using the wrapper is more consistent with all
the other places where requests are assigned.
Note that the whole point of the wrapper is that using it everywhere that
request pointers are assigned means that the reference counting is done
automatically and can't be accidentally forgotten about. Plus it allows simpler
future maintainance if the reference counting mechanisms ever need to change.
For: VIZ-4377
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/intel_display.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 91f0c19..43d8022 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9127,8 +9127,7 @@ static void intel_unpin_work_fn(struct work_struct
*__work)
intel_update_fbc(dev);
if (work->flip_queued_req)
- i915_gem_request_unreference(work->flip_queued_req);
- work->flip_queued_req = NULL;
+ i915_gem_request_assign(&work->flip_queued_req, NULL);
mutex_unlock(&dev->struct_mutex);
intel_frontbuffer_flip_complete(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
@@ -9626,10 +9625,9 @@ static void intel_mmio_flip_work_func(struct work_struct
*work)
intel_do_mmio_flip(crtc);
if (mmio_flip->req) {
mutex_lock(&crtc->base.dev->struct_mutex);
- i915_gem_request_unreference(mmio_flip->req);
+ i915_gem_request_assign(&mmio_flip->req, NULL);
mutex_unlock(&crtc->base.dev->struct_mutex);
}
- mmio_flip->req = NULL;
}
static int intel_queue_mmio_flip(struct drm_device *dev,
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/gem_userptr_blits: add test to check blit aligns
On Fri, Dec 05, 2014 at 03:32:30PM +0200, Mika Kuoppala wrote: > Copy a block into a destination object with varying base offset > into the destination and source. Put guard area before and after > the blit target to see that it didn't touch memory out of > blit boundaries. This doesn't belong in gem_userptr_blits. Try gem_basic_blt. -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] tests/gem_userptr_blits: add test to check blit aligns
Copy a block into a destination object with varying base offset
into the destination and source. Put guard area before and after
the blit target to see that it didn't touch memory out of
blit boundaries.
References: https://bugs.freedesktop.org/show_bug.cgi?id=79053
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
tests/gem_userptr_blits.c | 275 --
1 file changed, 268 insertions(+), 7 deletions(-)
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index d641c12..b945b48 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -81,6 +81,7 @@ static uint32_t userptr_flags =
LOCAL_I915_USERPTR_UNSYNCHRONIZED;
#define HEIGHT 512
static uint32_t linear[WIDTH*HEIGHT];
+static uint32_t gen;
static void gem_userptr_test_unsynchronized(void)
{
@@ -136,7 +137,7 @@ copy(int fd, uint32_t dst, uint32_t src, unsigned int error)
batch[i++] = XY_SRC_COPY_BLT_CMD |
XY_SRC_COPY_BLT_WRITE_ALPHA |
XY_SRC_COPY_BLT_WRITE_RGB;
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
batch[i - 1] |= 8;
else
batch[i - 1] |= 6;
@@ -147,12 +148,12 @@ copy(int fd, uint32_t dst, uint32_t src, unsigned int
error)
batch[i++] = 0; /* dst x1,y1 */
batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
batch[i++] = 0; /* dst reloc */
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
batch[i++] = 0;
batch[i++] = 0; /* src x1,y1 */
batch[i++] = WIDTH*4;
batch[i++] = 0; /* src reloc */
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
batch[i++] = 0;
batch[i++] = MI_BATCH_BUFFER_END;
batch[i++] = MI_NOOP;
@@ -170,7 +171,7 @@ copy(int fd, uint32_t dst, uint32_t src, unsigned int error)
reloc[1].target_handle = src;
reloc[1].delta = 0;
reloc[1].offset = 7 * sizeof(batch[0]);
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
reloc[1].offset += sizeof(batch[0]);
reloc[1].presumed_offset = 0;
reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
@@ -221,7 +222,7 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo,
int n_bo)
batch[i++] = XY_SRC_COPY_BLT_CMD |
XY_SRC_COPY_BLT_WRITE_ALPHA |
XY_SRC_COPY_BLT_WRITE_RGB;
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
batch[i - 1] |= 8;
else
batch[i - 1] |= 6;
@@ -231,7 +232,7 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo,
int n_bo)
batch[i++] = 0; /* dst x1,y1 */
batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
batch[i++] = 0; /* dst reloc */
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
batch[i++] = 0;
batch[i++] = 0; /* src x1,y1 */
batch[i++] = WIDTH*4;
@@ -254,7 +255,7 @@ blit(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo,
int n_bo)
reloc[1].target_handle = src;
reloc[1].delta = 0;
reloc[1].offset = 7 * sizeof(batch[0]);
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (gen >= 8)
reloc[1].offset += sizeof(batch[0]);
reloc[1].presumed_offset = 0;
reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
@@ -972,6 +973,258 @@ static int test_coherency(int fd, int count)
return 0;
}
+static uint32_t lb[12];
+
+static void
+copy_align(int fd,
+ uint32_t dst, uint32_t dst_offset,
+ uint32_t src, uint32_t src_offset,
+ uint32_t w, uint32_t h)
+{
+ uint32_t batch[12];
+ struct drm_i915_gem_relocation_entry reloc[2];
+ struct drm_i915_gem_exec_object2 obj[3];
+ struct drm_i915_gem_execbuffer2 exec;
+ uint32_t handle;
+ int i=0;
+
+ batch[i++] = XY_SRC_COPY_BLT_CMD |
+ XY_SRC_COPY_BLT_WRITE_ALPHA |
+ XY_SRC_COPY_BLT_WRITE_RGB;
+ if (gen >= 8)
+ batch[i - 1] |= 8;
+ else
+ batch[i - 1] |= 6;
+
+ batch[i++] = (3 << 24) | /* 32 bits */
+ (0xcc << 16) | /* copy ROP */
+ w * 4;
+
+ batch[i++] = 0; /* dst, x1,y2 */
+ batch[i++] = ((h) << 16) | (w); /* dst x2,y2 */
+ batch[i++] = dst_offset * 4; /* dst reloc */
+ if (gen >= 8)
+ batch[i++] = 0;
+ batch[i++] = 0; /* src x1,y1 */
+ batch[i++] = w * 4;
+
+ if ((w * 4) % 8)
+ igt_warn("src pitch NOT aligned to quad\n");
+
+ batch[i++] = src_offset * 4; /* src reloc */
+ if (gen >= 8)
+ batch[i++] = 0;
+ batch[i++] = MI_BATCH_BUFFER_END;
+ batch[i++] = MI_NOOP;
+
+ handle = gem_create(fd, 4096);
+ gem_write(fd, handle, 0, batch, sizeof(batch));
+
+ reloc[0].target_handle
[Intel-gfx] [PATCH] drm/i915: allow requesting the audio power well for all platforms
So far we only allowed HSW and BDW to request for the audio power
domain, but it is also needed at least on VLV/CHV. There is no need
for this restriction, since the power domain->power well mapping should
take care of the distinctions between platforms.
Spotted-by: Jani Nikula
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8a2bd18..58204ab 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -50,7 +50,7 @@
* present for a given platform.
*/
-static struct i915_power_domains *hsw_pwr;
+static struct i915_power_domains *i915_pwr;
#define for_each_power_well(i, power_well, domain_mask, power_domains) \
for (i = 0; \
@@ -1098,10 +1098,8 @@ int intel_power_domains_init(struct drm_i915_private
*dev_priv)
*/
if (IS_HASWELL(dev_priv->dev)) {
set_power_wells(power_domains, hsw_power_wells);
- hsw_pwr = power_domains;
} else if (IS_BROADWELL(dev_priv->dev)) {
set_power_wells(power_domains, bdw_power_wells);
- hsw_pwr = power_domains;
} else if (IS_CHERRYVIEW(dev_priv->dev)) {
set_power_wells(power_domains, chv_power_wells);
} else if (IS_VALLEYVIEW(dev_priv->dev)) {
@@ -1110,6 +1108,8 @@ int intel_power_domains_init(struct drm_i915_private
*dev_priv)
set_power_wells(power_domains, i9xx_always_on_power_well);
}
+ i915_pwr = power_domains;
+
return 0;
}
@@ -1146,7 +1146,7 @@ void intel_power_domains_fini(struct drm_i915_private
*dev_priv)
* we're going to unload/reload. */
intel_display_set_init_power(dev_priv, true);
- hsw_pwr = NULL;
+ i915_pwr = NULL;
}
static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
@@ -1360,10 +1360,10 @@ int i915_request_power_well(void)
{
struct drm_i915_private *dev_priv;
- if (!hsw_pwr)
+ if (!i915_pwr)
return -ENODEV;
- dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+ dev_priv = container_of(i915_pwr, struct drm_i915_private,
power_domains);
intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
return 0;
@@ -1375,10 +1375,10 @@ int i915_release_power_well(void)
{
struct drm_i915_private *dev_priv;
- if (!hsw_pwr)
+ if (!i915_pwr)
return -ENODEV;
- dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+ dev_priv = container_of(i915_pwr, struct drm_i915_private,
power_domains);
intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
return 0;
@@ -1395,10 +1395,10 @@ int i915_get_cdclk_freq(void)
{
struct drm_i915_private *dev_priv;
- if (!hsw_pwr)
+ if (!i915_pwr)
return -ENODEV;
- dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+ dev_priv = container_of(i915_pwr, struct drm_i915_private,
power_domains);
return intel_ddi_get_cdclk_freq(dev_priv);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Enable HDMI Clock recovery for Pipe C
On Fri, 2014-12-05 at 13:57 +0200, Jani Nikula wrote: > On Fri, 05 Dec 2014, Imre Deak wrote: > > On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote: > >> On Fri, 05 Dec 2014, Clint Taylor wrote: > >> > On 12/04/2014 11:08 AM, Clint Taylor wrote: > >> >> On 12/04/2014 12:41 AM, Jani Nikula wrote: > >> >>> On Wed, 03 Dec 2014, Clint Taylor wrote: > >> On 12/03/2014 01:01 PM, Ville Syrjälä wrote: > >> > On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.tay...@intel.com > >> > wrote: > >> >> From: Clint Taylor > >> >> > >> >> Added PIPE C register support for CHV audio programming. > >> > > >> > nak. The offset between the pipes looks constant so it should work > >> > just fine with _PIPE(). > >> > >> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared > >> after > >> ChromeOS boot. > >> >>> > >> >>> "after boot" is a long time! ;) > >> >>> > >> >>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets > >> >>> called? With drm.debug=14 you should see "Disable audio codec on port > >> >>> %c, pipe %c\n". > >> >>> > >> >>> BR, > >> >>> Jani. > >> >>> > >> >> > >> >> I've actually instrumented the driver with more logging and at 22.43 in > >> >> the dmesg log the last call to audio_codec_enable() is called and is > >> >> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for > >> >> 1080p60. Sometime after 22.436 and before the ChromeOS login screen > >> >> appears the register is being cleared to 0x0 without knowledge/action of > >> >> the i915 driver. Mode change and power state change seem to restore the > >> >> programming correctly. > >> >> > >> >> Most likely candidate right now is an ordering issue with snd_hda > >> >> during boot. > >> >> > >> > > >> > Ok, removing snd_hda drivers from the boot process did not change the > >> > result. Any ideas about what could reset the HD_AUDIO registers to their > >> > default? > >> > >> Maybe it needs some power domain/well we fail to get? Similar to what > >> DDI based platforms do before and after calls to > >> intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c. > >> > >> Not really my area, maybe Imre and Ville know better? > > > > The audio power domain->power well mapping is mapped correctly in the > > i915 driver (HDA belongs to the display aka pipe-A power well there). > > But AFAICS the intel_hda driver doesn't ask for this power domain on > > VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda > > device capability flags. CC'ing Takashi for this. > > Imre, i915_request_power_well() only works for hsw/bdw. Hm, right I should have checked that first. And it could be also the reason why AZX_DCAPS_I915_POWERWELL was left out for everything but HSW/BDW. I will send a patch to fix that in i915. Thanks, Imre > > BR, > Jani. > > > > > Still, I'm not sure why the register gets zeroed, if there is no power > > well off->on transition. You could first check if this is really true, > > via the debug print intel_display_power_get() and > > intel_display_power_put() emits. > > > > Another idea would be to trace writes to this register through > > I915_WRITE. > > > > --Imre > > > >> > >> > >> BR, > >> Jani. > >> > >> > >> > >> > > >> > -clint > >> > > >> > > >> >> [8.607022] [drm:intel_audio_codec_enable] ELD on > >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] > >> >> [8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port > >> >> D, pipe C, 8 bytes ELD > >> >> [8.607044] *** CAT enable aud_config reg 0x001E2200 = 0x0009 > >> >> [ 14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port > >> >> D, pipe C > >> >> [ 14.741055] *** CAT disable aud_config reg 0x001E2200 = 0x1000 > >> >> [ 15.773618] [drm:intel_audio_codec_enable] ELD on > >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] > >> >> [ 15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port > >> >> D, pipe C, 8 bytes ELD > >> >> [ 15.779315] *** CAT enable aud_config reg 0x001E2200 = 0x0009 > >> >> [ 19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port > >> >> D, pipe C > >> >> [ 19.936778] *** CAT disable aud_config reg 0x001E2200 = 0x1000 > >> >> [ 20.292920] [drm:intel_audio_codec_enable] ELD on > >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] > >> >> [ 20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port > >> >> D, pipe C, 8 bytes ELD > >> >> [ 20.298615] *** CAT enable aud_config reg 0x001E2200 = 0x0009 > >> >> [ 20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port > >> >> D, pipe C > >> >> [ 20.389121] *** CAT disable aud_config reg 0x001E2200 = 0x1000 > >> >> [ 22.430931] [drm:intel_audio_codec_enable] ELD on > >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36] > >> >> [ 22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port > >> >> D, pipe C, 8 bytes ELD > >> >> [
Re: [Intel-gfx] [PATCH -next] drm/i915: Fix missing unlock on error in i915_gem_init_hw()
On Fri, Dec 05, 2014 at 08:55:59AM +0800, weiyj...@163.com wrote:
> From: Wei Yongjun
>
> Add the missing unlock before return from function i915_gem_init_hw()
> in the error handling case.
>
> Signed-off-by: Wei Yongjun
Applied, thanks for the patch. Two minor comments:
- Please mention the commit that introduced the issue next time around.
I've added that while applying.
- The usual patter is
if (ret)
goto out;
/* more code */
out:
mutex_unlock();
return ret;
This would work really well in i915_gem_init_hw and besides the
code-cleanup also prevents such a fumble in the future. If you feel like
please submit that patch to convert init_hw to this shared unlock code
pattern, too.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d2ba315..3eeb2d0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4879,8 +4879,10 @@ i915_gem_init_hw(struct drm_device *dev)
> i915_gem_init_swizzling(dev);
>
> ret = dev_priv->gt.init_rings(dev);
> - if (ret)
> + if (ret) {
> + mutex_unlock(&dev->struct_mutex);
> return ret;
> + }
>
> for (i = 0; i < NUM_L3_SLICES(dev); i++)
> i915_gem_l3_remap(&dev_priv->ring[RCS], i);
>
> ___
> 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] igt/drm_read: Abuse read(drm)
Check that the more obvious userspace error conditions are handled by
the kernel, ideally without loss of data. These include nonblocking
waits, passing invalid buffers and passing buffers of the incorrect
length.
Signed-off-by: Chris Wilson
---
tests/.gitignore | 1 +
tests/Makefile.sources | 1 +
tests/drm_read.c | 192 +
3 files changed, 194 insertions(+)
create mode 100644 tests/drm_read.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 869a80c..38f6f96 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -4,6 +4,7 @@ core_getclient
core_getstats
core_getversion
drm_import_export
+drm_read
drm_vma_limiter
drm_vma_limiter_cached
drm_vma_limiter_cpu
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 42bfed9..da9e742 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs = \
core_getstats \
core_getversion \
drm_import_export \
+ drm_read \
drm_vma_limiter \
drm_vma_limiter_cached \
drm_vma_limiter_cpu \
diff --git a/tests/drm_read.c b/tests/drm_read.c
new file mode 100644
index 000..79cf64d
--- /dev/null
+++ b/tests/drm_read.c
@@ -0,0 +1,192 @@
+/*
+ * 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:
+ *Chris Wilson
+ *
+ */
+
+/*
+ * Testcase: boundary testing of read(drm_fd)
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION("Call read(drm) and see if it behaves.");
+
+static void sighandler(int sig)
+{
+}
+
+static void assert_empty(int fd)
+{
+ struct pollfd pfd = {fd, POLLIN };
+ igt_assert(poll(&pfd, 1, 0) == 0);
+}
+
+static void generate_event(int fd)
+{
+ union drm_wait_vblank vbl;
+
+ /* We assume that pipe 0 is running */
+
+ vbl.request.type =
+ DRM_VBLANK_RELATIVE |
+ DRM_VBLANK_EVENT;
+ vbl.request.sequence = 0;
+
+ igt_assert(drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0);
+}
+
+static void wait_for_event(int fd)
+{
+ struct pollfd pfd = {fd, POLLIN };
+ igt_assert(poll(&pfd, 1, -1) == 1);
+}
+
+static void teardown(int fd)
+{
+ alarm(0);
+ assert_empty(fd);
+ close(fd);
+ errno = 0;
+}
+
+static void test_invalid_buffer(int in)
+{
+ int fd = dup(in);
+ int ret;
+
+ alarm(1);
+ ret = read(fd, NULL, 4096);
+
+ igt_assert_eq(ret, -1);
+ igt_assert_eq(errno, EFAULT);
+
+ teardown(fd);
+}
+
+static void test_empty(int in, int nonblock, int expected)
+{
+ char buffer[1024];
+ int fd = dup(in);
+ int ret;
+
+ if (nonblock) {
+ ret = -1;
+ if (fd != -1)
+ ret = fcntl(fd, F_GETFL);
+ if (ret != -1) {
+ ret |= O_NONBLOCK;
+ ret = fcntl(fd, F_SETFL, ret);
+ }
+ igt_require(ret != -1);
+ }
+
+ assert_empty(fd);
+
+ alarm(1);
+ ret = read(fd, buffer, sizeof(buffer));
+ igt_assert_eq(ret, -1);
+ igt_assert_eq(errno, expected);
+
+ teardown(fd);
+}
+
+static void test_short_buffer(int in, int nonblock)
+{
+ char buffer[1024]; /* events are typically 32 bytes */
+ int fd = dup(in);
+ int ret;
+
+ if (nonblock) {
+ ret = -1;
+ if (fd != -1)
+ ret = fcntl(fd, F_GETFL);
+ if (ret != -1) {
+ ret |= O_NONBLOCK;
+ ret = fcntl(fd, F_SETFL, ret);
+ }
+ igt_require(ret != -1);
+ }
+
+
Re: [Intel-gfx] [RFC PATCH] drm/i915: avoid round-trip scaling errors in actual_brightness
On Tue, 02 Dec 2014, "Eoff, Ullysses A" wrote: > bump Talked about this with Daniel a while back, his take was, "preemptively catering to bonghits userspace means we'll have less wiggle-room in the future" BR, Jani. > >> -Original Message- >> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf >> Of Eoff, Ullysses A >> Sent: Thursday, November 20, 2014 9:34 AM >> To: Nikula, Jani; intel-gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [RFC PATCH] drm/i915: avoid round-trip scaling >> errors in actual_brightness >> >> > -Original Message- >> > From: Nikula, Jani >> > Sent: Thursday, November 20, 2014 3:59 AM >> > To: intel-gfx@lists.freedesktop.org >> > Cc: Nikula, Jani; Eoff, Ullysses A >> > Subject: [RFC PATCH] drm/i915: avoid round-trip scaling errors in >> > actual_brightness >> > >> > Due to scaling, the userspace and hardware brightness ranges might not >> > have a 1:1 mapping, causing the backlight class sysfs actual_brightness >> > not match the brightness attribute just written. While this is not a >> > strict requirement per Documentation/ABI/stable/sysfs-class-backlight, >> > try the userspace->hardware scaling for a match first and return the >> > cached value to not confuse userspace. >> > >> > The problem was already mitigated by >> > >> > commit 673e7bbdb3920b62cfc6c710bea626b0a9b0f43a >> > Author: U. Artie Eoff >> > Date: Mon Sep 29 15:49:32 2014 -0700 >> > >> > drm/i915: intel_backlight scale() math WA >> > >> > but this should be more robust for cases where the userspace expects >> > actual_brightness to match the just written brightness. >> > >> > Reference: >> > http://mid.gmane.org/1415737838-9640-1-git-send-email-ullysses.a.e...@intel.com >> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=85861 >> > Cc: U. Artie Eoff >> > Signed-off-by: Jani Nikula >> >> Tested-by: U. Artie Eoff >> >> > --- >> > drivers/gpu/drm/i915/intel_panel.c | 12 +++- >> > 1 file changed, 11 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_panel.c >> > b/drivers/gpu/drm/i915/intel_panel.c >> > index 4d63839bd9b4..675b56e105e7 100644 >> > --- a/drivers/gpu/drm/i915/intel_panel.c >> > +++ b/drivers/gpu/drm/i915/intel_panel.c >> > @@ -1024,7 +1024,17 @@ static int >> > intel_backlight_device_get_brightness(struct backlight_device *bd) >> >drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); >> > >> >hw_level = intel_panel_get_backlight(connector); >> > - ret = scale_hw_to_user(connector, hw_level, bd->props.max_brightness); >> > + >> > + /* >> > + * Check the userspace->hardware scaling for a match first to avoid >> > + * scaling errors in the userspace->hardware->userspace round-trip. >> > + */ >> > + if (hw_level == scale_user_to_hw(connector, bd->props.brightness, >> > + bd->props.max_brightness)) >> > + ret = bd->props.brightness; >> > + else >> > + ret = scale_hw_to_user(connector, hw_level, >> > + bd->props.max_brightness); >> > >> >drm_modeset_unlock(&dev->mode_config.connection_mutex); >> >intel_runtime_pm_put(dev_priv); >> > -- >> > 2.1.3 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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] [ANNOUNCE][RFC] KVMGT - the implementation of Intel GVT-g(full GPU virtualization) for KVM
On 05/12/2014 09:50, Gerd Hoffmann wrote: > A few comments on the kernel stuff (brief look so far, also > compile-tested only, intel gfx on my test machine is too old). > > * Noticed the kernel bits don't even compile when configured as >module. Everything (vgt, i915, kvm) must be compiled into the >kernel. I'll add that the patch is basically impossible to review with all the XenGT bits still in. For example, the x86 emulator seems to be unnecessary for KVMGT, but I am not 100% sure. I would like a clear understanding of why/how Andrew Barnes was able to do i915 passthrough (GVT-d) without hacking the ISA bridge, and why this does not apply to GVT-g. Paolo > * Design approach still seems to be i915 on vgt not the other way >around. > > Qemu/SeaBIOS bits: > > I've seen the host bridge changes identity from i440fx to > copy-pci-ids-from-host. Guess the reason for this is that seabios uses > this device to figure whenever it is running on i440fx or q35. Correct? > > What are the exact requirements for the device? Must it match the host > exactly, to not confuse the guest intel graphics driver? Or would > something more recent -- such as the q35 emulation qemu has -- be good > enough to make things work (assuming we add support for the > graphic-related pci config space registers there)? > > The patch also adds a dummy isa bridge at 0x1f. Simliar question here: > What exactly is needed here? Would things work if we simply use the q35 > lpc device here? > > more to come after I've read the paper linked above ... > > cheers, > Gerd > > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: release struct_mutex on the i915_gem_init_hw fail path
Jani,
On Fri, Dec 05, 2014 at 02:17:42PM +0200, Jani Nikula wrote:
> Release struct_mutex if init_rings() fails.
>
> This is a regression introduced in
> commit 35a57ffbb10840af219eeaf64718434242bb7c76
> Author: Daniel Vetter
> Date: Thu Nov 20 00:33:07 2014 +0100
>
> drm/i915: Only init engines once
>
> Reported-by: Wei Yongjun
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_gem.c | 16 +++-
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c1c11418231b..e3ce4bef22a3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4860,22 +4860,18 @@ int i915_gem_init(struct drm_device *dev)
> }
>
> ret = i915_gem_init_userptr(dev);
> - if (ret) {
> - mutex_unlock(&dev->struct_mutex);
> - return ret;
> - }
> + if (ret)
> + goto out_unlock;
>
> i915_gem_init_global_gtt(dev);
>
> ret = i915_gem_context_init(dev);
> - if (ret) {
> - mutex_unlock(&dev->struct_mutex);
> - return ret;
> - }
> + if (ret)
> + goto out_unlock;
>
> ret = dev_priv->gt.init_rings(dev);
> if (ret)
> - return ret;
> + goto out_unlock;
>
> ret = i915_gem_init_hw(dev);
> if (ret == -EIO) {
> @@ -4887,6 +4883,8 @@ int i915_gem_init(struct drm_device *dev)
> atomic_set_mask(I915_WEDGED,
> &dev_priv->gpu_error.reset_counter);
> ret = 0;
> }
> +
> +out_unlock:
> mutex_unlock(&dev->struct_mutex);
>
> return ret;
> --
> 2.1.3
>
Yes, this looks much better.
--
- Jeremiah Mahler
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 03/10] drm/i915: Add support for port enable/disable for dual link configuration
On Fri, 05 Dec 2014, Gaurav K Singh wrote:
> For Dual Link MIPI Panels, both Port A and Port C should be enabled
> during the MIPI encoder enabling sequence. Similarly, during the
> disabling sequence, both ports needs to be disabled.
>
> v2: Used for_each_dsi_port macro instead of for loop
>
> v3: Used intel_dsi->ports instead of dual_link var for dual link
> configuration check
>
> v4: Masking of the required MIPI port bits before writing proper values
>
> Signed-off-by: Gaurav K Singh
> Signed-off-by: Shobhit Kumar
> Reviewed-by: Jani Nikula
Yup.
> ---
> drivers/gpu/drm/i915/i915_reg.h|1 +
> drivers/gpu/drm/i915/intel_dsi.c | 37
> +++-
> drivers/gpu/drm/i915/intel_dsi.h |1 +
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |4 +++
> 4 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc03fac..c981f5d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6664,6 +6664,7 @@ enum punit_power_well {
> #define DPI_ENABLE (1 << 31) /* A + C */
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf << 27)
> +#define DUAL_LINK_MODE_SHIFT26
> #define DUAL_LINK_MODE_MASK (1 << 26)
> #define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
> #define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 << 26)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 693736b..fd4d397 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -108,28 +108,41 @@ static void intel_dsi_port_enable(struct intel_encoder
> *encoder)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
> + enum port port;
> u32 temp;
>
> - /* assert ip_tg_enable signal */
> - temp = I915_READ(MIPI_PORT_CTRL(port)) & ~LANE_CONFIGURATION_MASK;
> - temp = temp | intel_dsi->port_bits;
> - I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + for_each_dsi_port(port, intel_dsi->ports) {
> + temp = I915_READ(MIPI_PORT_CTRL(port));
> + temp &= ~LANE_CONFIGURATION_MASK;
> + temp &= ~DUAL_LINK_MODE_MASK;
> +
> + if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> + temp |= (intel_dsi->dual_link - 1)
> + << DUAL_LINK_MODE_SHIFT;
> + temp |= intel_crtc->pipe ?
> + LANE_CONFIGURATION_DUAL_LINK_B :
> + LANE_CONFIGURATION_DUAL_LINK_A;
> + }
> + /* assert ip_tg_enable signal */
> + I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> + POSTING_READ(MIPI_PORT_CTRL(port));
> + }
> }
>
> static void intel_dsi_port_disable(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> - enum port port = intel_dsi_pipe_to_port(intel_crtc->pipe);
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + enum port port;
> u32 temp;
>
> - /* de-assert ip_tg_enable signal */
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> - I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + for_each_dsi_port(port, intel_dsi->ports) {
> + /* de-assert ip_tg_enable signal */
> + temp = I915_READ(MIPI_PORT_CTRL(port));
> + I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
> + POSTING_READ(MIPI_PORT_CTRL(port));
> + }
> }
>
> static void intel_dsi_device_ready(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 7f5d028..f2cc2fc 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -104,6 +104,7 @@ struct intel_dsi {
> u8 clock_stop;
>
> u8 escape_clk_div;
> + u8 dual_link;
> u32 port_bits;
> u32 bw_timer;
> u32 dphy_reg;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 7766c42..f60146f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -287,6 +287,10 @@ static bool generic_init(struct intel_dsi_device *d
Re: [Intel-gfx] [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/
On Fri, Nov 14, 2014 at 06:16:56PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > MI_STORE_DWORD_IMM length has been the same ever since gen4. Rename > the define to avoid potential confusion if someone tries to use this > on pre-gen8. > > Also correct the comment on MI_MEM_VIRTUAL bit. It's present on 945,g33 > and 965 only. > > Cc: Oscar Mateo > Signed-off-by: Ville Syrjälä ping > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3de58ac..5228493 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -280,8 +280,8 @@ > #define MI_SEMAPHORE_POLL (1<<15) > #define MI_SEMAPHORE_SAD_GTE_SDD (1<<12) > #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1) > -#define MI_STORE_DWORD_IMM_GEN8 MI_INSTR(0x20, 2) > -#define MI_MEM_VIRTUAL (1 << 22) /* 965+ only */ > +#define MI_STORE_DWORD_IMM_GEN4 MI_INSTR(0x20, 2) > +#define MI_MEM_VIRTUAL (1 << 22) /* 945,g33,965 */ > #define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1) > #define MI_STORE_DWORD_INDEX_SHIFT 2 > /* Official intel docs are somewhat sloppy concerning MI_LOAD_REGISTER_IMM: > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 6025ac7..649d9ba 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1188,7 +1188,7 @@ static int gen8_emit_request(struct intel_ringbuffer > *ringbuf) > if (ret) > return ret; > > - cmd = MI_STORE_DWORD_IMM_GEN8; > + cmd = MI_STORE_DWORD_IMM_GEN4; > cmd |= MI_GLOBAL_GTT; > > intel_logical_ring_emit(ringbuf, cmd); > -- > 2.0.4 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller
On 12/4/2014 4:52 PM, Daniel Vetter wrote: On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote: On Thu, 04 Dec 2014, Gaurav K Singh wrote: This patch is in preparation of DSI dual link panels. For dual link panels, few packets needs to be sent to Port A or Port C or both. Based on the portno from MIPI Sequence Block#53, these sequences needs to be sent accordingly. v2: Addressed review comments by Jani - port variables named properly Signed-off-by: Gaurav K Singh Reviewed-by: Jani Nikula Merged the first two patches, thanks. -Daniel Hi Daniel, Thanks. I have addressed Jani's review comments too for the rest of the patches. With regards, Gaurav ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: release struct_mutex on the i915_gem_init_hw fail path
Release struct_mutex if init_rings() fails.
This is a regression introduced in
commit 35a57ffbb10840af219eeaf64718434242bb7c76
Author: Daniel Vetter
Date: Thu Nov 20 00:33:07 2014 +0100
drm/i915: Only init engines once
Reported-by: Wei Yongjun
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_gem.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c1c11418231b..e3ce4bef22a3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4860,22 +4860,18 @@ int i915_gem_init(struct drm_device *dev)
}
ret = i915_gem_init_userptr(dev);
- if (ret) {
- mutex_unlock(&dev->struct_mutex);
- return ret;
- }
+ if (ret)
+ goto out_unlock;
i915_gem_init_global_gtt(dev);
ret = i915_gem_context_init(dev);
- if (ret) {
- mutex_unlock(&dev->struct_mutex);
- return ret;
- }
+ if (ret)
+ goto out_unlock;
ret = dev_priv->gt.init_rings(dev);
if (ret)
- return ret;
+ goto out_unlock;
ret = i915_gem_init_hw(dev);
if (ret == -EIO) {
@@ -4887,6 +4883,8 @@ int i915_gem_init(struct drm_device *dev)
atomic_set_mask(I915_WEDGED,
&dev_priv->gpu_error.reset_counter);
ret = 0;
}
+
+out_unlock:
mutex_unlock(&dev->struct_mutex);
return ret;
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915: Fix missing unlock on error in i915_gem_init_hw()
On Fri, 05 Dec 2014, Jeremiah Mahler wrote: > There are other places in i915_gem_init_hw() where it returns without > unlocking the mutex. Why is it only necessary here and not any of the > other places? There's probably some rebase/merge confusion going on. The following patch is against drm-intel-next-queued where the problem is present. Thanks for reporting this. BR, Jani. Jani Nikula (1): drm/i915: release struct_mutex on the i915_gem_init_hw fail path drivers/gpu/drm/i915/i915_gem.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915: Fix missing unlock on error in i915_gem_init_hw()
On Fri, 05 Dec 2014, Jeremiah Mahler wrote: > There are other places in i915_gem_init_hw() where it returns without > unlocking the mutex. Why is it only necessary here and not any of the > other places? There's probably some rebase/merge confusion going on. The following patch is against drm-intel-next-queued where the problem is present. Thanks for reporting this. BR, Jani. Jani Nikula (1): drm/i915: release struct_mutex on the i915_gem_init_hw fail path drivers/gpu/drm/i915/i915_gem.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
