Re: [Intel-gfx] [PATCH v2 6/6] drm/i915/vlv: Modifying WA 'WaDisableL3Bank2xClockGate for vlv
On Mon, 2014-03-24 at 17:56 +, Lespiau, Damien wrote: On Mon, Mar 24, 2014 at 11:00:07PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com For disabling L3 clock gating we need to set bit 25 of MMIO register 940c. Earlier this was being done by just writing 1 into bit 25 and resetting all other bits. This patch modifies the routine to read-modify-write of the register, so that the values of other bits are not destroyed. v2: Modifying the comments and the patch commit message (Chris) This patch commit message lacks the most important information: which bit are we setting back to 0 and we shouldn't, and why is that important? We do direct writes to other registers in that function (for instance (MI_ARB_VLV just below). Hi Damien, The reset value of the register is 0x00F80003. Therefore, if we directly set only bit 25 to 1, without caring about other bits, the following reg bits will be affected (bits 1:0, bits 23:19). This doesn't seem to be the case with other regs where we are writing directly (MI_ARB_VLV ) whose default value is 0. So, by this commit we're just trying to set only the bit which we really want to change. Regards, Sourab ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully. If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness. Other than the major performance degredation due to our implementation, and that there is a known deadlock (when unplugging/plugging in external displays) due to the broken locking... It should not have been enabled. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: use hrtimer in wait for vblank
On Tue, Mar 25, 2014 at 11:29:02AM +0530, Arun R Murthy wrote: In wait for vblank use usleep_range, which will use hrtimers instead of msleep. Using msleep(1~20) there are more chances of sleeping for 20ms. Using usleep_range uses hrtimers and hence are precise, worst case will trigger an interrupt at the higher/max timeout. Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have noticed the time consumed by wait for vblank is ~4ms to ~17ms. Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae Signed-off-by: Arun R Murthy arun.r.mur...@intel.com No. I feel strongly that we do not want more wait_for_X() with strange semantics. http://sweng.the-davies.net/Home/rustys-api-design-manifesto -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Broadwell expands ACTHD to 64bit
On Mon, Mar 24, 2014 at 07:43:48PM -0700, Ben Widawsky wrote:
On Mon, Mar 24, 2014 at 07:41:17PM -0700, Ben Widawsky wrote:
On Fri, Mar 21, 2014 at 12:41:53PM +, Chris Wilson wrote:
As Broadwell has an increased virtual address size, it requires more
than 32 bits to store offsets into its address space. This includes the
debug registers to track the current HEAD of the individual rings, which
may be anywhere within the per-process address spaces. In order to find
the full location, we need to read the high bits from a second register.
We then also need to expand our storage to keep track of the larger
address.
v2: Carefully read the two registers to catch wraparound between
the reads.
v3: Use a WARN_ON rather than loop indefinitely on an unstable
register read.
I truly feel bad for saying this at v3, but we can probably simplify
this. Unless I am missing something, we only actually care about the
value of acthd in:
if (ring-hangcheck.acthd != acthd)
return HANGCHECK_ACTIVE;
I think therefore it would be safe to make hangcheck.acthd a u64.
Compare the lower dword. If they're not equal, then we're done. If they
are equal, compare the UDW. If UDW doesn't match, we're done. If UDW
does match, do another read of the LDW and call it good:
ring_stuck(u32 acthd)
{
if (lower_32_bits(ring-hangcheck.acthd) != acthd)
return HANGCHECK_ACTIVE;
else if (upper_32_bits(ring-hangcheck.acthd) !=
I915_READ(ACTHD_UDW))
return HANGCHECK_ACTIVE
else if (acthd != I915_READ(RING_ACTHD))
return HANGCHECK_ACTIVE;
}
After writing that, I'm not really sure how much less complex it is, but I
think it gets the same job done. Potentially there is some MMIO savings,
but I'd guess it to be negligible.
Jesse, please request ACTHD is expanded to a proper 64b register for
gen1.
Right after I sent that, I realized that doesn't provide for potentially
corrupting ring-hangcheck.acthd. So I am back to thinking this method
is better.
Plus having read64_2x32 should make it easier to dtrt next time.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use the DVI clock limit in DVI mode
On Tue, Mar 25, 2014 at 2:44 AM, Stéphane Marchesin
marc...@chromium.org wrote:
When using HDMI, the 300MHz clock is legal, but when in DVI mode it's
definitely not. This causes issues when we send a 300MHz signal over a
DVI cable which is specced for 165MHz only. So when in DVI mode let's
limit the clock to 165MHz.
Signed-off-by: Stéphane Marchesin marc...@chromium.org
Is
commit 6375b768a9850b6154478993e5fb566fa4614a9c
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Mon Mar 3 11:33:36 2014 +0200
drm/i915: Reject 165MHz modes w/ DVI monitors
not working for you?
-Daniel
---
drivers/gpu/drm/i915/intel_hdmi.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index dd4fa35..0ac69f1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -806,6 +806,10 @@ static int hdmi_portclock_limit(struct intel_hdmi *hdmi)
{
struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+ /* If we are in DVI mode, the limit is 165MHz */
+ if (!hdmi-has_hdmi_sink)
+ return 165000;
+
if (IS_G4X(dev))
return 165000;
else if (IS_HASWELL(dev))
--
1.9.1.423.g4596e3a
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.
This should prevent UXA from getting stuck in a null render loop after a
failed resume.
Reported-by: Jiri Kosina ji...@jikos.cz
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b45163e19f3..22c650490f54 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -991,6 +991,18 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
}
+static bool
+intel_ring_valid(struct intel_ring_buffer *ring)
+{
+ switch (ring-id) {
+ case RCS: return true;
+ case VCS: return HAS_BSD(ring-dev);
+ case BCS: return HAS_BLT(ring-dev);
+ case VECS: return HAS_VEBOX(ring-dev);
+ default: return false;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1041,7 +1053,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!intel_ring_initialized(ring)) {
DRM_DEBUG(execbuf with invalid ring: %d\n,
(int)(args-flags I915_EXEC_RING_MASK));
- return -EINVAL;
+ return intel_ring_valid(ring) ? -EIO : -EINVAL;
}
mode = args-flags I915_EXEC_CONSTANTS_MASK;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 9:03 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Feels a bit like duct-tape ... Shouldn't we instead stop tearing down ringbuffer structures over suspend/resume? And maybe the same for init with your patch applied. Or we simply check for wedged first thing in execbuf ... -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: Enable FBC on GEN7 by default
On Tue, Mar 25, 2014 at 8:27 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully. If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness. Other than the major performance degredation due to our implementation, and that there is a known deadlock (when unplugging/plugging in external displays) due to the broken locking... It should not have been enabled. Also, have you run the full kms_fbc_crc testsuite to make sure it's actually functionally correct? Iirc we even fail at that stage still on some platforms ... -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: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 09:07:00AM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 9:03 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Feels a bit like duct-tape ... Shouldn't we instead stop tearing down ringbuffer structures over suspend/resume? And maybe the same for init with your patch applied. Even if we did, we would still end up with g45 failing to restart the ring at random, so we need some w/a. Or we simply check for wedged first thing in execbuf ... See the first 2 patches ;-) The first is actually essential as we have no other guard against writing into the non-existent ring. I thought about doing that. However, I prefer doing arg validation first i.e. get all (or as much as is feasible) of the EINVAL checks out of the way so that we avoid touching hardware or leaking any information to a malicious client. The problem in this case is where is not actually an invalid arg. Note that we will detect the EIO later before touching hardware (so long as the first two patches are in place). -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 v5] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
From: Akash Goel akash.g...@intel.com
Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
This workaround has to be applied before doing TLB Invalidation.
In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
Store data commands.
Without this, hardware cannot guarantee the command after the PIPE_CONTROL
with TLB inv will not use the old TLB values.
v2: Modified the WA comment (Ville)
v3: Added the vlv identifier with WA name (Damien)
v4: Reworked based on Chris' comments (WA moved to gen7 ring flush func,
sending 6 dwords instead of 8) (Chris)
v5: Enhancing the scope of WA to gen6, gen7. Having a common WA func being
called from gen6, gen7 flush functions. (Ville)
Signed-off-by: Sourab Gupta sourab.gu...@intel.com
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 52 +
1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 87d1a2d..ef4ca3dd 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -208,6 +208,30 @@ intel_emit_post_sync_nonzero_flush(struct
intel_ring_buffer *ring)
}
static int
+gen6_tlb_invalidate_wa(struct intel_ring_buffer *ring)
+{
+ /*
+* WaTlbInvalidateStoreDataBefore:gen6,gen7
+* This workaround has to be applied before doing TLB invalidation.
+* Before pipecontrol with TLB invalidate set, need 2 store
+* data commands (such as MI_STORE_DATA_IMM or MI_STORE_DATA_INDEX)
+* Without this, hardware cannot guarantee the command after the
+* PIPE_CONTROL with TLB inv will not use the old TLB values.
+*/
+ int i, ret;
+ ret = intel_ring_begin(ring, 3 * 2);
+ if (ret)
+ return ret;
+ for (i = 0; i 2; i++) {
+ intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
+ intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR);
+ intel_ring_emit(ring, 0);
+ }
+ intel_ring_advance(ring);
+ return 0;
+}
+
+static int
gen6_render_ring_flush(struct intel_ring_buffer *ring,
u32 invalidate_domains, u32 flush_domains)
{
@@ -215,6 +239,13 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
u32 scratch_addr = ring-scratch.gtt_offset + 128;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate_domains) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
+
/* Force SNB workarounds for PIPE_CONTROL flushes */
ret = intel_emit_post_sync_nonzero_flush(ring);
if (ret)
@@ -309,6 +340,13 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
u32 scratch_addr = ring-scratch.gtt_offset + 128;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate_domains) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
+
/*
* Ensure that any following seqno writes only happen when the render
* cache is indeed flushed.
@@ -1733,6 +1771,13 @@ static int gen6_bsd_ring_flush(struct intel_ring_buffer
*ring,
uint32_t cmd;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
@@ -1837,6 +1882,13 @@ static int gen6_ring_flush(struct intel_ring_buffer
*ring,
uint32_t cmd;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: use hrtimer in wait for vblank
In wait for vblank use usleep_range, which will use hrtimers instead of
msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
Using usleep_range uses hrtimers and hence are precise, worst case will
trigger an interrupt at the higher/max timeout.
As per kernel document Documentation/timers/timers-howto.txt sleeping
for 10us to 20ms its recomended to use usleep_range.
Signed-off-by: Arun R Murthy arun.r.mur...@intel.com
---
drivers/gpu/drm/i915/intel_drv.h |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..29a8664 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -52,7 +52,10 @@
break; \
} \
if (W drm_can_sleep()) {\
- msleep(W); \
+ if (W 20) \
+ msleep(W); \
+ else\
+ usleep_range(W * 1000, W * 2 * 1000); \
} else {\
cpu_relax();\
} \
--
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] drm/i915: use hrtimer in wait for vblank
On Tue, Mar 25, 2014 at 02:28:22PM +0530, Arun R Murthy wrote: In wait for vblank use usleep_range, which will use hrtimers instead of msleep. Using msleep(1~20) there are more chances of sleeping for 20ms. Using usleep_range uses hrtimers and hence are precise, worst case will trigger an interrupt at the higher/max timeout. As per kernel document Documentation/timers/timers-howto.txt sleeping for 10us to 20ms its recomended to use usleep_range. Signed-off-by: Arun R Murthy arun.r.mur...@intel.com Lgtm, I still feel that our use of W=1 is fairly arbitrary and worth tweaking in future. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -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 v5] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
On Tue, Mar 25, 2014 at 02:01:05PM +0530, sourab.gu...@intel.com wrote:
From: Akash Goel akash.g...@intel.com
Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
This workaround has to be applied before doing TLB Invalidation.
In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
Store data commands.
Without this, hardware cannot guarantee the command after the PIPE_CONTROL
with TLB inv will not use the old TLB values.
v2: Modified the WA comment (Ville)
v3: Added the vlv identifier with WA name (Damien)
v4: Reworked based on Chris' comments (WA moved to gen7 ring flush func,
sending 6 dwords instead of 8) (Chris)
v5: Enhancing the scope of WA to gen6, gen7. Having a common WA func being
called from gen6, gen7 flush functions. (Ville)
@@ -1733,6 +1771,13 @@ static int gen6_bsd_ring_flush(struct
intel_ring_buffer *ring,
uint32_t cmd;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
BSD uses MI_FLUSH_DW. Does this w/a still apply? Do we need it for BLT
as well? VEBOX?
-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 v4] drm/i915: use hrtimer in wait for vblank
On Tue, 25 Mar 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Mar 25, 2014 at 02:28:22PM +0530, Arun R Murthy wrote: In wait for vblank use usleep_range, which will use hrtimers instead of msleep. Using msleep(1~20) there are more chances of sleeping for 20ms. Using usleep_range uses hrtimers and hence are precise, worst case will trigger an interrupt at the higher/max timeout. As per kernel document Documentation/timers/timers-howto.txt sleeping for 10us to 20ms its recomended to use usleep_range. Signed-off-by: Arun R Murthy arun.r.mur...@intel.com Lgtm, I still feel that our use of W=1 is fairly arbitrary and worth tweaking in future. With the current code, this is essentially the same as the original patch. We never have W 20, and thus we always take the usleep_range() path. So W is definitely worth tweaking if we go with this now. Nitpick, the macro params should be parenthesized. This will now break for _wait_for(cond, 10, 2 + 1) and such. Arun, please don't immediately reply with updated patches if there's discussion still going on. See what the conclusion is first. Thanks. BR, Jani. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre -- 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 v5] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
On Tue, 2014-03-25 at 09:15 +, Chris Wilson wrote:
On Tue, Mar 25, 2014 at 02:01:05PM +0530, sourab.gu...@intel.com wrote:
From: Akash Goel akash.g...@intel.com
Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
This workaround has to be applied before doing TLB Invalidation.
In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
Store data commands.
Without this, hardware cannot guarantee the command after the PIPE_CONTROL
with TLB inv will not use the old TLB values.
v2: Modified the WA comment (Ville)
v3: Added the vlv identifier with WA name (Damien)
v4: Reworked based on Chris' comments (WA moved to gen7 ring flush func,
sending 6 dwords instead of 8) (Chris)
v5: Enhancing the scope of WA to gen6, gen7. Having a common WA func being
called from gen6, gen7 flush functions. (Ville)
@@ -1733,6 +1771,13 @@ static int gen6_bsd_ring_flush(struct
intel_ring_buffer *ring,
uint32_t cmd;
int ret;
+ /* Apply WaTlbInvalidateStoreDataBefore workaround */
+ if (invalidate) {
+ ret = gen6_tlb_invalidate_wa(ring);
+ if (ret)
+ return ret;
+ }
BSD uses MI_FLUSH_DW. Does this w/a still apply? Do we need it for BLT
as well? VEBOX?
-Chris
This should be applicable only to the render ring funcs, not the bsd,
blt, vebox ones, as you called out. PIPE_CONTROL is used only in render
ring, others use MI_FLUSH. My bad for missing this out.
Thanks,
Sourab
___
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: fix up semaphore_waits_for
On Mon, Mar 24, 2014 at 03:37:12PM -0700, Ben Widawsky wrote: On Sat, Mar 22, 2014 at 06:52:25PM +0100, Daniel Vetter wrote: On Fri, Mar 21, 2014 at 07:33:59PM +0200, Mika Kuoppala wrote: Hi, Daniel Vetter daniel.vet...@ffwll.ch writes: There's an entire pile of issues in here: - Use the main RING_HEAD register, not ACTHD. ACTHD points at the gtt offset of the batch buffer when a batch is executed. Semaphores are always emitted to the main ring, so we always want to look at that. The ipehr check should make sure that we are at the ring and acthd should not be at batch. Regardless I agree that RING_HEAD is much more safer. When I have tried to get bottom of the snb reset hang, I have noticed that after reset the acthd is sometimes 0x0c even tho head is 0x00, on snb. Hm, should we maybe mask out the lower bits, too? If you can confirm this, can you please add a follow-up patch? - Mask the obtained HEAD pointer with the actual ring size, which is much smaller. Together with the above issue this resulted us in trying to dereference a pointer way outside of the ring mmio mapping. The resulting invalid access in interrupt context (hangcheck is executed from timers) lead to a full blown kernel panic. The fbcon panic handler then tried to frob our driver harder, resulting in a full machine hang at least on my snb here where I've stumbled over this. - Handle ring wrapping correctly and be a bit more explicit about how many dwords we're scanning. We probably should also scan more than just 4 ... ipehr dont update on MI_NOOPS and the head should point to the extra MI_NOOP after semaphore sequence. So 4 should be enough. Perhaps revisit this when bdw gains semaphores. Yeah, Chris also mentioned that the HEAD should point at one dword past the end of the ring, so should even work when there are no MI_NOOPs. In any case this code is more robust in case hw engineers suddenly change the behaviour. - Space out some of teh computations for readability. This prevents hard-hangs on my snb here. Verdict from QA is still pending, but the symptoms match. Cc: Mika Kuoppala mika.kuopp...@intel.com Cc: Ben Widawsky b...@bwidawsk.net Cc: Chris Wilson ch...@chris-wilson.co.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74100 The hard hangs are not so frequent with this patch but they are still there. This patch should take care of the oops we have been seeing, but there is still something else to be found until #74100 can be marked as fixed. Very often after reset, when igt has pushed the quietence batches into rings, blitter and video ring heads gets moved properly but all updates to hardware status page and to the sync registers are missing. And result is hang by hangcheck. Repeat enough times and it is a hard hang. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Please remove the blowup comment and also update the bugzilla tag. Yeah, QA also says that it doesn't fix the hard hangs, only seems to reduce them a bit on certain setups. I've updated the commit message. btw are you testing with FBDEV=n? The lack of a fbcon panic handler should greatly increase the chances that the last few message get correctly transmitted through other means like netconsole. Patches 1-2 and the followup one are Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com Thanks for the review, patches merged. -Daniel Patch 2 was trivial to implement for gen8. This functionality is a lot less trivial. I volunteered to do patch 2, are you going to port this to gen8? If you fill out the bdw pieces for patch 23 the only thing you need to change here is to adjsut the number of dwords we walk backwards to make sure that the bdw semaphore cmd still fits. Or at least that's been my idea behind all this, maybe I've overlooked something. -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 00/12] Broadwell 3.14 backports
On Mon, Mar 24, 2014 at 04:17:42PM -0700, Ben Widawsky wrote: On Mon, Mar 24, 2014 at 04:14:32PM -0700, Ausmus, James wrote: On Sat, Mar 22, 2014 at 4:34 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Mar 21, 2014 at 05:51:01PM -0700, Ben Widawsky wrote: Let's try this again. I've pushed a branch here: http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=bdw-backports I need to re-review some of the merge conflicts for 4g GGTT, which I will try to do before Monday. Daniel: please make sure this is what you had in mind. I don't know where you want Cc: stable tags. We don't need cc: stable, we just need to submit it (since it has the upstream sha1s already, which is the requirement for stable patches). Cc: stable is only for when you want it to get backport anyway. Otherwise looks good. I dunno whether git cherry-pick can be told to use the sha1 reference layout Greg prefers or not, he uses This is sha1 in upstream. between the commit header and the actual commit message. But ime his scripts reformat your commit messages anyway. James: please test this as soon as possible. Once this is tested and we conclude it's sufficient to get bdw going on 3.14 without hilarity I think we should do a quick review on intel-gfx to check that the impact outside of bdw is indeed minimal. Then once drm-next has landed with all the referenced commits we can submit it to Greg with a small cover letter why we want this and that plan B would be to kill bdw in 3.14. This seems to be working well for me, with the one caveat that on boot and once per resume I'm hitting the WARN(!i915_preliminary_hw_support, GEN8_CENTROID_PIXEL_OPT_DIS not be needed for production) code in gen8_init_clock_gating - can that WARN be dropped via drm/i915: Don't use i915_preliminary_hw_support to mean pre-production ? Both with and without that patch added, the series is: Tested-by: James Ausmus james.aus...@intel.com Thanks. Daniel, the patch is added to my backports branch. I think given that that it removes a WARN which we know to be bogus, it's a good patch for stable. But it's your call. Oh, that patch definitely should go to stable if we fix up bdw in 3.14 ;-) -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 full PPGTT with param override
On Mon, Mar 24, 2014 at 06:06:00PM -0700, Ben Widawsky wrote: When PPGTT was disabled by default, the patch also prevented the user from overriding this behavior via module parameter. Being able to test this on arbitrary kernels is extremely beneficial to track down the remaining bugs. The patch that prevented this was: commit 93a25a9e2d67765c3092bfaac9b855d95e39df97 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Mar 6 09:40:43 2014 +0100 drm/i915: Disable full ppgtt by default By default PPGTT is set to -1. 0 means off, 1 means aliasing only, 2 means full, all other values are reserved. Signed-off-by: Ben Widawsky b...@bwidawsk.net My apologies for breaking this a bit harder than intended, and thanks for fixing it up. Patch merged to dinq. -Daniel --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index bbc9420..9068628 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -50,7 +50,7 @@ bool intel_enable_ppgtt(struct drm_device *dev, bool full) /* Full ppgtt disabled by default for now due to issues. */ if (full) - return false; /* HAS_PPGTT(dev) */ + return HAS_PPGTT(dev) (i915.enable_ppgtt == 2); else return HAS_ALIASING_PPGTT(dev); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add property to set HDMI aspect ratio
On Tue, Mar 25, 2014 at 09:47:01AM +0530, Vandana Kannan wrote:
Added a property to enable user space to set aspect ratio for HDMI displays.
If there is no user specified value, then PAR_NONE/Automatic option is set
by default. User can select aspect ratio 4:3 or 16:9. The aspect ratio
selected by user would come into effect with a mode set.
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Jesse Barnes jesse.bar...@intel.com
Cc: Vijay Purushothaman vijay.a.purushotha...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Traditionally we do a full modeset to update the property when it is set.
That leads to a redundant modeset right now, but that should go away again
once we have all the atomic stuff. And doing the full modeset is needed
since we don't track this state, and the next modeset might get optimized
to a simple pageflip. Which means there's no easy way to set this value.
For examples see the other property implementations.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h|1 +
drivers/gpu/drm/i915/intel_drv.h |2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 10 ++
drivers/gpu/drm/i915/intel_modes.c | 28
4 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b8c1e0..628ba2e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1588,6 +1588,7 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ struct drm_property *aspect_ratio_property;
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index fa99104..262142f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -474,6 +474,7 @@ struct intel_hdmi {
bool has_audio;
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
+ enum hdmi_picture_aspect aspect_ratio;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
@@ -834,6 +835,7 @@ int intel_connector_update_modes(struct drm_connector
*connector,
int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter
*adapter);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_aspect_ratio_property(struct drm_connector *connector);
/* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index b0413e1..ae7628e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct
drm_encoder *encoder,
union hdmi_infoframe frame;
int ret;
+ /* Set user selected PAR to incoming mode's member */
+ adjusted_mode-picture_aspect_ratio = intel_hdmi-aspect_ratio;
+
ret = drm_hdmi_avi_infoframe_from_display_mode(frame.avi,
adjusted_mode);
if (ret 0) {
@@ -1094,6 +1097,9 @@ intel_hdmi_set_property(struct drm_connector *connector,
goto done;
}
+ if (property == dev_priv-aspect_ratio_property)
+ intel_hdmi-aspect_ratio = val;
+
return -EINVAL;
done:
@@ -1227,6 +1233,7 @@ intel_hdmi_add_properties(struct intel_hdmi
*intel_hdmi, struct drm_connector *c
{
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
+ intel_attach_aspect_ratio_property(connector);
intel_hdmi-color_range_auto = true;
}
@@ -1291,6 +1298,9 @@ void intel_hdmi_init_connector(struct
intel_digital_port *intel_dig_port,
intel_connector-get_hw_state = intel_connector_get_hw_state;
intel_connector-unregister = intel_connector_unregister;
+ /* Initialize aspect ratio member of intel_hdmi */
+ intel_hdmi-aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
intel_hdmi_add_properties(intel_hdmi, connector);
intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_modes.c
b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..6f814da 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,31 @@ intel_attach_broadcast_rgb_property(struct drm_connector
*connector)
drm_object_attach_property(connector-base, prop, 0);
}
+
+static const struct drm_prop_enum_list aspect_ratio_names[] = {
+ { HDMI_PICTURE_ASPECT_NONE, Automatic },
+ { HDMI_PICTURE_ASPECT_4_3, 4:3 },
+ { HDMI_PICTURE_ASPECT_16_9, 16:9 },
Re: [Intel-gfx] [PATCH v2] drm/i915: use hrtimer in wait for vblank
On Tue, Mar 25, 2014 at 11:25:03AM +0530, Arun R Murthy wrote:
BZ: 178761
In wait for vblank use usleep_range, which will use hrtimers instead of
msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
Using usleep_range uses hrtimers and hence are precise, worst case will
trigger an interrupt at the higher/max timeout.
Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have
noticed the time consumed by wait for vblank is ~4ms to ~17ms.
Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae
Signed-off-by: Arun R Murthy arun.r.mur...@intel.com
Do we actually have that many vblank waits in time critical sections
still? If we can't get rid of then and they need to be faster the right
approach imo is to replace the wait loops with interrupt waits (with still
a fallback timeout after 50ms or so of course) using our drm vblank
infrastructure.
That will wake up at exactly the time we want to, without any unnecessary
wakeups in between or other ugly overhead.
So as-is with the justification of optimizing vblank waits the wait_for_us
optimization is nacked.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c |2 +-
drivers/gpu/drm/i915/intel_dp.c |4 ++--
drivers/gpu/drm/i915/intel_drv.h | 19 ---
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..9de2678 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -761,7 +761,7 @@ static void g4x_wait_for_vblank(struct drm_device *dev,
int pipe)
frame = I915_READ(frame_reg);
- if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
+ if (wait_for_us(I915_READ_NOTRACE(frame_reg) != frame, 50, 1000))
DRM_DEBUG_KMS(vblank wait timed out\n);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1ef3d4..14927e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1074,7 +1074,7 @@ static void wait_panel_status(struct intel_dp *intel_dp,
I915_READ(pp_stat_reg),
I915_READ(pp_ctrl_reg));
- if (_wait_for((I915_READ(pp_stat_reg) mask) == value, 5000, 10)) {
+ if (wait_for_ms((I915_READ(pp_stat_reg) mask) == value, 5000, 10)) {
DRM_ERROR(Panel status timeout: status %08x control %08x\n,
I915_READ(pp_stat_reg),
I915_READ(pp_ctrl_reg));
@@ -1808,7 +1808,7 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
I915_READ(EDP_PSR_CTL(dev)) ~EDP_PSR_ENABLE);
/* Wait till PSR is idle */
- if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev))
+ if (wait_for_ms((I915_READ(EDP_PSR_STATUS_CTL(dev))
EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
DRM_ERROR(Timed out waiting for PSR Idle State\n);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..bbda97e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -42,8 +42,8 @@
* having timed out, since the timeout could be due to preemption or similar
and
* we've never had a chance to check the condition before the timeout.
*/
-#define _wait_for(COND, MS, W) ({ \
- unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1; \
+#define _wait_for(COND, TIMEOUT, MS, US) ({ \
+ unsigned long timeout__ = jiffies + msecs_to_jiffies(TIMEOUT) + 1;\
int ret__ = 0; \
while (!(COND)) { \
if (time_after(jiffies, timeout__)) { \
@@ -51,8 +51,11 @@
ret__ = -ETIMEDOUT; \
break; \
} \
- if (W drm_can_sleep()) {\
- msleep(W); \
+ if ((MS | US) drm_can_sleep()) {\
+ if (MS) \
+ msleep(MS); \
+ else\
+ usleep_range(US, US * 2); \
} else {\
cpu_relax();\
} \
@@ -60,10 +63,12 @@
ret__; \
})
-#define wait_for(COND, MS) _wait_for(COND, MS, 1)
-#define
Re: [Intel-gfx] [PATCH 0/4] Adding support for plane alpha/color blending through drm property
Hi Damien, Could you please clarify following queries. Thanks, Sagar On Fri, 2014-03-21 at 19:06 +0530, Sagar Arun Kamble wrote: Hi Damien, On Thu, 2014-03-20 at 14:45 +, Damien Lespiau wrote: On Thu, Mar 20, 2014 at 02:11:40PM +, Damien Lespiau wrote: (source is premultiplied) RGBA = ADD(SRC_COLOR*SRC_ALPHA, DST_COLOR*ONE_MINUS_SRC_ALPHA) Grr, copy/paste error. If the source is already premultiplied: RGBA = ADD(SRC_COLOR, DST_COLOR*ONE_MINUS_SRC_ALPHA) 1. Currently there is no interface to advertise the DRM_FORMATS plane supportes to user mode? Should we add new IOCTL for that and include pre-multiplied alpha formats while advertising? Or am I missing any such API already available? 2. About constant alpha property - when we program constant alpha register will hardware be able to take care of the blending as per equations you have specified for non-premultiplied-alpha and premultiplied-alpha cases or we have to do any additional setting? Confusion is because of two combinations: a. pre-multiplied alpha+constant alpha b. non-pre-multiplied alpha+constant alpha Kindly clarify. thanks, Sagar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/1] drm/i915: Fixing cursor size parameters for wm calculation
From: Sagar Kamble sagar.a.kam...@intel.com Cursor size is changed now take care of larger cursor sizes. wm calculation was hardcoded to 64 before so changing it. Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Jani Nikula jani.nik...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ad58ce3..2177e3d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2120,7 +2120,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, p-pri.bytes_per_pixel = crtc-fb-bits_per_pixel / 8; p-cur.bytes_per_pixel = 4; p-pri.horiz_pixels = intel_crtc-config.pipe_src_w; - p-cur.horiz_pixels = 64; + p-cur.horiz_pixels = dev-mode_config.cursor_width; /* TODO: for now, assume primary and cursor planes are always enabled. */ p-pri.enabled = true; p-cur.enabled = true; -- 1.8.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] Adding support for plane alpha/color blending through drm property
On Tue, Mar 25, 2014 at 03:33:42PM +0530, Sagar Arun Kamble wrote: Hi Damien, Could you please clarify following queries. He did, in a reply to your mail ... -Daniel Thanks, Sagar On Fri, 2014-03-21 at 19:06 +0530, Sagar Arun Kamble wrote: Hi Damien, On Thu, 2014-03-20 at 14:45 +, Damien Lespiau wrote: On Thu, Mar 20, 2014 at 02:11:40PM +, Damien Lespiau wrote: (source is premultiplied) RGBA = ADD(SRC_COLOR*SRC_ALPHA, DST_COLOR*ONE_MINUS_SRC_ALPHA) Grr, copy/paste error. If the source is already premultiplied: RGBA = ADD(SRC_COLOR, DST_COLOR*ONE_MINUS_SRC_ALPHA) 1. Currently there is no interface to advertise the DRM_FORMATS plane supportes to user mode? Should we add new IOCTL for that and include pre-multiplied alpha formats while advertising? Or am I missing any such API already available? 2. About constant alpha property - when we program constant alpha register will hardware be able to take care of the blending as per equations you have specified for non-premultiplied-alpha and premultiplied-alpha cases or we have to do any additional setting? Confusion is because of two combinations: a. pre-multiplied alpha+constant alpha b. non-pre-multiplied alpha+constant alpha Kindly clarify. thanks, Sagar ___ 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 v6] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'. This workaround has to be applied before doing TLB Invalidation on render ring. In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI Store data commands. Without this, hardware cannot guarantee the command after the PIPE_CONTROL with TLB inv will not use the old TLB values. Note, that our command programming sequence already has multiple dword writes between the flush of the last batch and the invalidation of the next. Is this w/a still required? Why? -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 v2 4/6] drm/i915/vlv: Remove the enabling of VS_TIMER_DISPATCH bit in MI MODE reg
On Mon, Mar 24, 2014 at 11:58:22PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com Removing the VS_TIMER_DISPATCH bit enable for MI MODE reg for Gen7 platform as it is not required. v2: Enhancing the scope of the patch to full Gen7 (Chris) Signed-off-by: Akash Goel akash.g...@intel.com Signed-off-by: Sourab Gupta sourab.gu...@intel.com Tested-by: Chris Wilson ch...@chris-wilson.co.uk # ivb, hsw -Chris --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index eb4811a..9983802 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -599,7 +599,9 @@ static int init_render_ring(struct intel_ring_buffer *ring) int ret = init_ring_common(ring); if (INTEL_INFO(dev)-gen 3) - I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH)); + if (!IS_GEN7(dev)) We shouldn't enable this on gen8 either, and while doing that you could avoid the extra indentation by rewriting it as something like this: if (INTEL_INFO(dev)-gen = 4 INTEL_INFO(dev)-gen 7) Also you could add the appropriate w/a note while you're touching the code: WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb + I915_WRITE(MI_MODE, + _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH)); /* We need to disable the AsyncFlip performance optimisations in order * to use MI_WAIT_FOR_EVENT within the CS. It should already be -- 1.8.5.1 -- 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 0/4] Adding support for plane alpha/color blending through drm property
On Tue, Mar 25, 2014 at 11:51:57AM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 03:33:42PM +0530, Sagar Arun Kamble wrote: Hi Damien, Could you please clarify following queries. He did, in a reply to your mail ... In case you cannot find it: http://lists.freedesktop.org/archives/intel-gfx/2014-March/042136.html -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 4/6] drm/i915: Remove the enabling of VS_TIMER_DISPATCH bit in MI MODE reg
From: Akash Goel akash.g...@intel.com This patch Removes the VS_TIMER_DISPATCH bit enable in MI MODE reg for platforms Gen6. VS_TIMER_DISPATCH bit enable was earlier required as a part of WA 'WaTimedSingleVertexDispatch', which is now applicable only to platforms Gen7. v2: Enhancing the scope of the patch to full Gen7 (Chris) v3: Modifying the WA condition to the cover the applicable platforms, and adding the WA name in comments. (Ville) Signed-off-by: Akash Goel akash.g...@intel.com Signed-off-by: Sourab Gupta sourab.gu...@intel.com Tested-by: Chris Wilson ch...@chris-wilson.co.uk # ivb, hsw -Chris --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 816137f..2ad5fe7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -605,7 +605,8 @@ static int init_render_ring(struct intel_ring_buffer *ring) struct drm_i915_private *dev_priv = dev-dev_private; int ret = init_ring_common(ring); - if (INTEL_INFO(dev)-gen 3) + /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */ + if (INTEL_INFO(dev)-gen = 4 INTEL_INFO(dev)-gen 7) I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH)); /* We need to disable the AsyncFlip performance optimisations in order -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/6] drm/i915: Remove the enabling of VS_TIMER_DISPATCH bit in MI MODE reg
On Tue, Mar 25, 2014 at 06:01:50PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com This patch Removes the VS_TIMER_DISPATCH bit enable in MI MODE reg for platforms Gen6. VS_TIMER_DISPATCH bit enable was earlier required as a part of WA 'WaTimedSingleVertexDispatch', which is now applicable only to platforms Gen7. v2: Enhancing the scope of the patch to full Gen7 (Chris) v3: Modifying the WA condition to the cover the applicable platforms, and adding the WA name in comments. (Ville) Signed-off-by: Akash Goel akash.g...@intel.com Signed-off-by: Sourab Gupta sourab.gu...@intel.com Tested-by: Chris Wilson ch...@chris-wilson.co.uk # ivb, hsw -Chris Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 816137f..2ad5fe7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -605,7 +605,8 @@ static int init_render_ring(struct intel_ring_buffer *ring) struct drm_i915_private *dev_priv = dev-dev_private; int ret = init_ring_common(ring); - if (INTEL_INFO(dev)-gen 3) + /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */ + if (INTEL_INFO(dev)-gen = 4 INTEL_INFO(dev)-gen 7) I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH)); /* We need to disable the AsyncFlip performance optimisations in order -- 1.8.5.1 -- 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 00/13] Gen7 batch buffer command parser
On Thu, Mar 20, 2014 at 04:43:05PM +0200, Jani Nikula wrote: Hi Bradley - Apologies for my procrastination with the review; I don't easily recall as tedious a review as the command and register tables. And I sure have reviewed a lot of miserable stuff in the past. Most infuriatingly, I did not find a single real bug in the code! I think we'll need to automate some things going forward, for example listing the non-conforming length encoding with Damien's tools for cross checking. There are a few subtle points we need to discuss (separate mails internally) but all in all this series is: Reviewed-by: Jani Nikula jani.nik...@intel.com Ok, pulled this one in, thanks a lot for the patchesreview. I think it's time we start to move on with the next bits, the batch copy stuff seams like a suitable piece. There's still issues with launching the entire thing in the end, but we can start with the copy infrastructure. Open issues I see still: - The littel issue we're discussing internally. Like I've said that one is blocking us and needs to be resolved before we can switch to enforcing mode. - Secure batch dispatch is still fubar. - CodingStyle says: Functions should be a) at most 3 indent levels b) at most 3 ansi screens long (i.e. 75 lines). i915_parse_cmds violates both metrics pretty deftly. I think a few refactoring patches to extract helper functions and structure the flow a bit would be good. 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 5/6] tests/gem_exec_parse: Test for batches w/o MI_BATCH_BUFFER_END
On Thu, Jan 30, 2014 at 11:46:15AM +, Chris Wilson wrote:
On Wed, Jan 29, 2014 at 10:10:47PM +, Chris Wilson wrote:
On Wed, Jan 29, 2014 at 01:58:29PM -0800, bradley.d.vol...@intel.com wrote:
From: Brad Volkin bradley.d.vol...@intel.com
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
tests/gem_exec_parse.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 9e90408..004c3bf 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -257,6 +257,15 @@ igt_main
-EINVAL));
}
+ igt_subtest(batch-without-end) {
+ uint32_t noop[1024] = { 0 };
+ igt_assert(
+exec_batch(fd, handle,
+ noop, sizeof(noop),
+ I915_EXEC_RENDER,
+ -EINVAL));
Cheekier would be
uint32_t empty[] = { MI_NOOP, MI_NOOP, MI_BATCH_BUFFER_END, 0 };
for_each_ring() {
igt_assert(exec_batch(fd, handle, empty, sizeof(empty), ring, 0));
igt_assert(exec_batch(fd, handle, empty, 8, ring, -EINVAL));
}
On this subject, it should be
{ INVALID, INVALID, NOOP, NOOP, END, 0}
assert(exec(0, 4) == -EINVAL);
assert(exec(0, 8) == -EINVAL);
assert(exec(0, 12) == -EINVAL);
assert(exec(4, 8) == -EINVAL);
assert(exec(4, 12) == 0);
assert(exec(8, 12) == 0);
Brad, care to throw this nasties into the test pond, too?
-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 00/13] Gen7 batch buffer command parser
On Tue, 25 Mar 2014, Daniel Vetter dan...@ffwll.ch wrote: On Thu, Mar 20, 2014 at 04:43:05PM +0200, Jani Nikula wrote: Hi Bradley - Apologies for my procrastination with the review; I don't easily recall as tedious a review as the command and register tables. And I sure have reviewed a lot of miserable stuff in the past. Most infuriatingly, I did not find a single real bug in the code! I think we'll need to automate some things going forward, for example listing the non-conforming length encoding with Damien's tools for cross checking. There are a few subtle points we need to discuss (separate mails internally) but all in all this series is: Reviewed-by: Jani Nikula jani.nik...@intel.com Ok, pulled this one in, thanks a lot for the patchesreview. I think it's time we start to move on with the next bits, the batch copy stuff seams like a suitable piece. There's still issues with launching the entire thing in the end, but we can start with the copy infrastructure. Open issues I see still: - The littel issue we're discussing internally. Like I've said that one is blocking us and needs to be resolved before we can switch to enforcing mode. - Secure batch dispatch is still fubar. - CodingStyle says: Functions should be a) at most 3 indent levels b) at most 3 ansi screens long (i.e. 75 lines). i915_parse_cmds violates both metrics pretty deftly. I think a few refactoring patches to extract helper functions and structure the flow a bit would be good. Just extracting the handlers for (desc-flags CMD_DESC_REGISTER) and (desc-flags CMD_DESC_BITMASK) would go a long way. BR, Jani. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Refactor common lock handling between shrinker count/scan
We can share a few lines of tricky lock handling we need to use for both
shrinker routines and in the process fix the return value for count()
when reporting a deadlock.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem.c | 42 +
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 219fe35f9c45..135ee8bd55f6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4913,6 +4913,22 @@ static bool mutex_is_locked_by(struct mutex *mutex,
struct task_struct *task)
#endif
}
+static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
+{
+ if (!mutex_trylock(dev-struct_mutex)) {
+ if (!mutex_is_locked_by(dev-struct_mutex, current))
+ return false;
+
+ if (to_i915(dev)-mm.shrinker_no_lock_stealing)
+ return false;
+
+ *unlock = false;
+ } else
+ *unlock = true;
+
+ return true;
+}
+
static int num_vma_bound(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
@@ -4932,18 +4948,11 @@ i915_gem_shrinker_count(struct shrinker *shrinker,
struct shrink_control *sc)
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_device *dev = dev_priv-dev;
struct drm_i915_gem_object *obj;
- bool unlock = true;
unsigned long count;
+ bool unlock;
- if (!mutex_trylock(dev-struct_mutex)) {
- if (!mutex_is_locked_by(dev-struct_mutex, current))
- return 0;
-
- if (dev_priv-mm.shrinker_no_lock_stealing)
- return 0;
-
- unlock = false;
- }
+ if (!i915_gem_shrinker_lock(dev, unlock))
+ return 0;
count = 0;
list_for_each_entry(obj, dev_priv-mm.unbound_list, global_list)
@@ -5031,17 +5040,10 @@ i915_gem_shrinker_scan(struct shrinker *shrinker,
struct shrink_control *sc)
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_device *dev = dev_priv-dev;
unsigned long freed;
- bool unlock = true;
+ bool unlock;
- if (!mutex_trylock(dev-struct_mutex)) {
- if (!mutex_is_locked_by(dev-struct_mutex, current))
- return SHRINK_STOP;
-
- if (dev_priv-mm.shrinker_no_lock_stealing)
- return SHRINK_STOP;
-
- unlock = false;
- }
+ if (!i915_gem_shrinker_lock(dev, unlock))
+ return SHRINK_STOP;
freed = i915_gem_purge(dev_priv, sc-nr_to_scan);
if (freed sc-nr_to_scan)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Include bound and active pages in the count of shrinkable objects
When the machine is under a lot of memory pressure and being stressed by
multiple GPU threads, we quite often report fewer than shrinker-batch
(i.e. SHRINK_BATCH) pages to be freed. This causes the shrink_control to
skip calling into i915.ko to release pages, despite the GPU holding onto
most of the physical pages in its active lists.
References: https://bugs.freedesktop.org/show_bug.cgi?id=72742
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_dma.c | 8
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 42 +++--
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4e0a26a83500..5a37c75f4b3d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1739,8 +1739,8 @@ out_power_well:
intel_power_domains_remove(dev_priv);
drm_vblank_cleanup(dev);
out_gem_unload:
- if (dev_priv-mm.inactive_shrinker.scan_objects)
- unregister_shrinker(dev_priv-mm.inactive_shrinker);
+ if (dev_priv-mm.shrinker.scan_objects)
+ unregister_shrinker(dev_priv-mm.shrinker);
if (dev-pdev-msi_enabled)
pci_disable_msi(dev-pdev);
@@ -1791,8 +1791,8 @@ int i915_driver_unload(struct drm_device *dev)
i915_teardown_sysfs(dev);
- if (dev_priv-mm.inactive_shrinker.scan_objects)
- unregister_shrinker(dev_priv-mm.inactive_shrinker);
+ if (dev_priv-mm.shrinker.scan_objects)
+ unregister_shrinker(dev_priv-mm.shrinker);
io_mapping_free(dev_priv-gtt.mappable);
arch_phys_wc_del(dev_priv-gtt.mtrr);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a7ad864f1154..cb4bb171e6cc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -986,7 +986,7 @@ struct i915_gem_mm {
/** PPGTT used for aliasing the PPGTT with the GTT */
struct i915_hw_ppgtt *aliasing_ppgtt;
- struct shrinker inactive_shrinker;
+ struct shrinker shrinker;
bool shrinker_no_lock_stealing;
/** LRU list of objects with fence regs on them. */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c23034021753..219fe35f9c45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -54,9 +54,9 @@ static void i915_gem_object_update_fence(struct
drm_i915_gem_object *obj,
struct drm_i915_fence_reg *fence,
bool enable);
-static unsigned long i915_gem_inactive_count(struct shrinker *shrinker,
+static unsigned long i915_gem_shrinker_count(struct shrinker *shrinker,
struct shrink_control *sc);
-static unsigned long i915_gem_inactive_scan(struct shrinker *shrinker,
+static unsigned long i915_gem_shrinker_scan(struct shrinker *shrinker,
struct shrink_control *sc);
static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long
target);
static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
@@ -4651,10 +4651,10 @@ i915_gem_load(struct drm_device *dev)
dev_priv-mm.interruptible = true;
- dev_priv-mm.inactive_shrinker.scan_objects = i915_gem_inactive_scan;
- dev_priv-mm.inactive_shrinker.count_objects = i915_gem_inactive_count;
- dev_priv-mm.inactive_shrinker.seeks = DEFAULT_SEEKS;
- register_shrinker(dev_priv-mm.inactive_shrinker);
+ dev_priv-mm.shrinker.scan_objects = i915_gem_shrinker_scan;
+ dev_priv-mm.shrinker.count_objects = i915_gem_shrinker_count;
+ dev_priv-mm.shrinker.seeks = DEFAULT_SEEKS;
+ register_shrinker(dev_priv-mm.shrinker);
}
/*
@@ -4913,13 +4913,23 @@ static bool mutex_is_locked_by(struct mutex *mutex,
struct task_struct *task)
#endif
}
+static int num_vma_bound(struct drm_i915_gem_object *obj)
+{
+ struct i915_vma *vma;
+ int count = 0;
+
+ list_for_each_entry(vma, obj-vma_list, vma_link)
+ if (drm_mm_node_allocated(vma-node))
+ count++;
+
+ return count;
+}
+
static unsigned long
-i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc)
+i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
{
struct drm_i915_private *dev_priv =
- container_of(shrinker,
-struct drm_i915_private,
-mm.inactive_shrinker);
+ container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_device *dev = dev_priv-dev;
struct drm_i915_gem_object *obj;
bool unlock = true;
@@ -4941,10 +4951,8 @@ i915_gem_inactive_count(struct shrinker *shrinker,
struct shrink_control *sc)
[Intel-gfx] [PATCH 1/4] drm/i915: Translate ENOSPC from shmem_get_page() to ENOMEM
shmemfs first checks if there is enough memory to allocate the page and reports ENOSPC should there be insufficient, along with the usual ENOMEM for a genuine allocation failure. We use ENOSPC in our driver to mean that we have run out of aperture space and so want to translate the error from shmemfs back to our usual understanding of ENOMEM. None of the the other GEM users appear to distinguish between ENOMEM and ENOSPC in their error handling, hence it is easiest to do the fixup in i915.ko Cc: Hugh Dickins hu...@google.com Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 33bbaa0d4412..c23034021753 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1976,7 +1976,19 @@ err_pages: page_cache_release(sg_page_iter_page(sg_iter)); sg_free_table(st); kfree(st); - return PTR_ERR(page); + + /* shmemfs first checks if there is enough memory to allocate the page +* and reports ENOSPC should there be insufficient, along with the usual +* ENOMEM for a genuine allocation failure. +* +* We use ENOSPC in our driver to mean that we have run out of aperture +* space and so want to translate the error from shmemfs back to our +* usual understanding of ENOMEM. +*/ + if (PTR_ERR(page) == -ENOSPC) + return -ENOMEM; + else + return PTR_ERR(page); } /* Ensure that the associated pages are gathered from the backing storage -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] tests/gem_exec_parse: Test a command crossing a page boundary
On Wed, Jan 29, 2014 at 10:12:08PM +, Chris Wilson wrote: On Wed, Jan 29, 2014 at 01:58:30PM -0800, bradley.d.vol...@intel.com wrote: From: Brad Volkin bradley.d.vol...@intel.com This is a speculative test in that it's not particularly relevant today, but is important if we switch the parser implementation to use kmap_atomic instead of vmap. Do you not want to iterate over all (or some combination of) valid/invalid commands to better fuzz the handling of boundaries? I think we can look into that once we decide that kmap_atomic is indeed the right way forward. This here seems good enough to at least have the basics ready for a quick test. Pulled in all six patches into igt, I think adding some of the additional cases Chris suggested for invalid handling might be useful. 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] tests/gem_exec_parse: fixups for the recent massive refactoring
I think we might have some use for a do_ioctl_expected_errno or some such thing. But that's for later. Cc: Brad Volkin bradley.d.vol...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- tests/gem_exec_parse.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c index 455bfbf4b22e..34d097d7199d 100644 --- a/tests/gem_exec_parse.c +++ b/tests/gem_exec_parse.c @@ -25,9 +25,12 @@ #include stdlib.h #include stdint.h #include stdio.h -#include drm.h -#include i915_drm.h +#include errno.h + +#include drm.h + #include drmtest.h +#include ioctl_wrappers.h #ifndef I915_PARAM_CMD_PARSER_VERSION #define I915_PARAM_CMD_PARSER_VERSION 28 -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Invalidate our pages under memory pressure
Try to flush out dirty pages into the swapcache (and from there into the
swapfile) when under memory pressure and forced to drop GEM objects from
memory. In effect, this should just allow us to discard unused pages for
memory reclaim and to start writeback earlier.
v2: Hugh Dickins warned that explicitly starting writeback from
shrink_slab was prone to deadlocks within shmemfs.
Cc: Hugh Dickins hu...@google.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem.c | 38 +++---
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 135ee8bd55f6..8287fd6701c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -60,7 +60,6 @@ static unsigned long i915_gem_shrinker_scan(struct shrinker
*shrinker,
struct shrink_control *sc);
static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long
target);
static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
-static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
static void i915_gem_retire_requests_ring(struct intel_ring_buffer *ring);
static bool cpu_cache_is_coherent(struct drm_device *dev,
@@ -1685,12 +1684,16 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void
*data,
return i915_gem_mmap_gtt(file, dev, args-handle, args-offset);
}
+static inline int
+i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
+{
+ return obj-madv == I915_MADV_DONTNEED;
+}
+
/* Immediately discard the backing storage */
static void
i915_gem_object_truncate(struct drm_i915_gem_object *obj)
{
- struct inode *inode;
-
i915_gem_object_free_mmap_offset(obj);
if (obj-base.filp == NULL)
@@ -1701,16 +1704,28 @@ i915_gem_object_truncate(struct drm_i915_gem_object
*obj)
* To do this we must instruct the shmfs to drop all of its
* backing pages, *now*.
*/
- inode = file_inode(obj-base.filp);
- shmem_truncate_range(inode, 0, (loff_t)-1);
-
+ shmem_truncate_range(file_inode(obj-base.filp), 0, (loff_t)-1);
obj-madv = __I915_MADV_PURGED;
}
-static inline int
-i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj)
+/* Try to discard unwanted pages */
+static void
+i915_gem_object_invalidate(struct drm_i915_gem_object *obj)
{
- return obj-madv == I915_MADV_DONTNEED;
+ struct address_space *mapping;
+
+ switch (obj-madv) {
+ case I915_MADV_DONTNEED:
+ i915_gem_object_truncate(obj);
+ case __I915_MADV_PURGED:
+ return;
+ }
+
+ if (obj-base.filp == NULL)
+ return;
+
+ mapping = file_inode(obj-base.filp)-i_mapping,
+ invalidate_mapping_pages(mapping, 0, (loff_t)-1);
}
static void
@@ -1775,8 +1790,7 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
ops-put_pages(obj);
obj-pages = NULL;
- if (i915_gem_object_is_purgeable(obj))
- i915_gem_object_truncate(obj);
+ i915_gem_object_invalidate(obj);
return 0;
}
@@ -4201,6 +4215,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
if (WARN_ON(obj-pages_pin_count))
obj-pages_pin_count = 0;
+ if (obj-madv != __I915_MADV_PURGED)
+ obj-madv = I915_MADV_DONTNEED;
i915_gem_object_put_pages(obj);
i915_gem_object_free_mmap_offset(obj);
i915_gem_object_release_stolen(obj);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 22/48] drm/i915: Use drm_mm for PPGTT PDEs
On Mon, Mar 24, 2014 at 01:02:57PM -0700, Ben Widawsky wrote:
On Mon, Mar 24, 2014 at 07:45:56PM +, Chris Wilson wrote:
On Mon, Mar 24, 2014 at 12:36:23PM -0700, Ben Widawsky wrote:
On Thu, Mar 20, 2014 at 11:10:13AM +, Chris Wilson wrote:
On Fri, Dec 06, 2013 at 02:11:55PM -0800, Ben Widawsky wrote:
static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
{
+#define GEN6_PD_ALIGN (PAGE_SIZE * 16)
+#define GEN6_PD_SIZE (GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE)
struct drm_device *dev = ppgtt-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- unsigned first_pd_entry_in_global_pt;
- int i;
- int ret = -ENOMEM;
+ int i, ret;
- /* ppgtt PDEs reside in the global gtt pagetable, which has
512*1024
- * entries. For aliasing ppgtt support we just steal them at
the end for
- * now. */
- first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt);
+ /* PPGTT PDEs reside in the GGTT and consists of 512 entries.
The
+ * allocator works in address space sizes, so it's multiplied
by page
+ * size. We allocate at the top of the GTT to avoid
fragmentation.
+ */
+ BUG_ON(!drm_mm_initialized(dev_priv-gtt.base.mm));
+ ret =
drm_mm_insert_node_in_range_generic(dev_priv-gtt.base.mm,
+ ppgtt-node,
GEN6_PD_SIZE,
+ GEN6_PD_ALIGN, 0,
+ 0,
dev_priv-gtt.base.total,
+
DRM_MM_SEARCH_DEFAULT);
This could use the simpler drm_mm_insert_node_generic().
-Chris
Not with my [simple] workaround to not use offset 0, which Daniel
reverted. I think he has some hope that we'll actually be able to figure
out why we can't use offset 0 instead of just using the workaround.
You can simply reduce the drm_mm range...
-Chris
Yeah, that's a better solution. Patches welcome?
I thought I had mentioned this earlier, but alas couldn't find it in
email. So here goes,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ee53551..0124e2c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1227,7 +1227,9 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
if (!ret) {
struct drm_i915_private *dev_priv = dev-dev_private;
kref_init(ppgtt-ref);
- drm_mm_init(ppgtt-base.mm, ppgtt-base.start,
+ /* Magic offset because we have no idea what's going on */
+ drm_mm_init(ppgtt-base.mm,
+ ppgtt-base.start + 4096,
ppgtt-base.total);
i915_init_vm(dev_priv, ppgtt-base);
if (INTEL_INFO(dev)-gen 8) {
--
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 1/1] kms_blend: tests blending of sprite planes using drm_property blend.
From: arsharma ankitprasad.r.sha...@intel.com
v1: This test currently tests constant alpha setting of sprite planes.
It verifies alpha setting of 0 and 255 with CRC.
Signed-off-by: Sharma, Ankitprasad R ankitprasad.r.sha...@intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
tests/Makefile.sources | 1 +
tests/kms_blend.c | 560 +
2 files changed, 561 insertions(+)
create mode 100644 tests/kms_blend.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 88866ac..b8a19cd 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -55,6 +55,7 @@ TESTS_progs_M = \
gem_tiled_partial_pwrite_pread \
gem_write_read_ring_switch \
kms_addfb \
+ kms_blend \
kms_cursor_crc \
kms_fbc_crc \
kms_flip \
diff --git a/tests/kms_blend.c b/tests/kms_blend.c
new file mode 100644
index 000..1990c52
--- /dev/null
+++ b/tests/kms_blend.c
@@ -0,0 +1,560 @@
+/*
+ * 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.
+ *
+ * Author:
+ * Ankitprasad Sharma ankitprasad.r.sharma at intel.com
+ * Sagar Kamble sagar.a.kam...@intel.com
+ */
+
+
+#include errno.h
+#include stdbool.h
+#include stdio.h
+#include string.h
+#include cairo.h
+#include math.h
+#include drm_fourcc.h
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_kms.h
+
+#define DRM_BLEND_NUM 24
+#define DRM_BLEND_PROP_SRC_COLOR 2
+#define DRM_BLEND_PROP_CONST_ALPHA 12
+#define BIT(x) (1 x)
+
+typedef struct {
+ struct kmstest_connector_config config;
+ drmModeModeInfo mode;
+ struct kmstest_fb fb[DRM_BLEND_NUM];
+ struct kmstest_fb sprite_fb[DRM_BLEND_NUM]
+} connector_t;
+
+typedef struct {
+ int drm_fd;
+ igt_debugfs_t debugfs;
+ drmModeRes *resources;
+ igt_crc_t ref_crtc_crc[DRM_BLEND_NUM];
+ igt_crc_t ref_sprite_crc[DRM_BLEND_NUM];
+ igt_pipe_crc_t **pipe_crc;
+ uint32_t crtc_id;
+ uint32_t sprite_id;
+ uint32_t crtc_idx;
+ uint32_t crtc_fb_id[DRM_BLEND_NUM];
+ uint32_t sprite_fb_id[DRM_BLEND_NUM];
+} data_t;
+
+static void set_blend(int drm_fd, uint32_t plane_id, uint64_t blend_prop,
double blend_val)
+{
+ int i = 0, j = 0, ret = 0;
+ uint64_t prop_blend;
+ uint64_t value, blend_val_int;
+ double blend_val_ex;
+ drmModeObjectPropertiesPtr props = NULL;
+
+ props = drmModeObjectGetProperties(drm_fd, plane_id,
DRM_MODE_OBJECT_PLANE);
+
+ switch(blend_prop)
+ {
+ case BIT(DRM_BLEND_PROP_CONST_ALPHA):
+ blend_val_ex = blend_val * 255;
+ blend_val_int = (uint64_t) blend_val_ex;
+ value = (blend_val_int 32);
+ prop_blend = value | blend_prop;
+ fprintf(stdout, blend_val_ex = %f, blend_val_int = %d,
value = %016llx, blend_prop = %016llx\n,
+ blend_val_ex, blend_val_int, value, blend_prop);
+ break;
+ case BIT(DRM_BLEND_PROP_SRC_COLOR):
+ prop_blend = blend_prop;
+ break;
+ default:
+ fprintf(stdout, Blend Type not supported\n);
+ return;
+ }
+
+ for (i = 0; i props-count_props; i++)
+ {
+ drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
props-props[i]);
+ fprintf(stdout, \nProp-name=%s , prop-name);
+
+ if (strcmp(prop-name, blend) == 0)
+ {
+ igt_assert(prop-flags (DRM_MODE_PROP_BITMASK |
DRM_MODE_PROP_32BIT_PAIR));
+ fprintf(stdout, \nBlending property enum count %d,
prop-count_enums);
+
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Fixing cursor size parameters for wm calculation
On Tue, Mar 25, 2014 at 03:46:36PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com Cursor size is changed now take care of larger cursor sizes. wm calculation was hardcoded to 64 before so changing it. Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Jani Nikula jani.nik...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ad58ce3..2177e3d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2120,7 +2120,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, p-pri.bytes_per_pixel = crtc-fb-bits_per_pixel / 8; p-cur.bytes_per_pixel = 4; p-pri.horiz_pixels = intel_crtc-config.pipe_src_w; - p-cur.horiz_pixels = 64; + p-cur.horiz_pixels = dev-mode_config.cursor_width; Hum? It seems that mode_config.cursor_width is supposed to have the preferred cursor size so a generic DDX (for instance) can select a size that can be used. If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. A previous patch from you doesn't seem to to do that correctly by always setting the max size, we're fine with the default 64x64 (I just sent a patch to address that). Instead of using the DRM cursor_config value the natural thing to do would be to use the actual width being used (sent patch). -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] A few cursor fixups after the multi-size patches
Noticed that, if I'm not mistaken, we were misunderstanding the intention of
mode_config.cursor_{width,height}, and those 3 patches are the resulting fixes.
Damien Lespiau (3):
drm/i915: Don't set mode_config's cursor size
drm/i915: Remove max_cursor_{width,height} from the crtc
drm/i915: Use the actual cursor width for WM computation
drivers/gpu/drm/i915/intel_display.c | 10 --
drivers/gpu/drm/i915/intel_drv.h | 7 ---
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 1 insertion(+), 18 deletions(-)
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/4] drm/i915: Enabling constant alpha drm property
From: Sagar Kamble sagar.a.kam...@intel.com
This patch enables constant alpha property for Sprite planes.
Client has to set BIT(DRM_BLEND_CONSTANT_ALPHA) | ((alpha value) 32)
for applying constant alpha on a plane. To disable constant alpha,
client has to set BIT(DRM_BLEND_SRC_COLOR)
v2: Fixing property value comparison in vlv_update_plane with the use
of BIT(). Replacing DRM_BLEND_NONE by DRM_BLEND_SRC_COLOR.
Reverting line spacing change in i915_driver_lastclose. Return value
of intel_plane_set_property is changed to -ENVAL [Damien's Review Comments]
Testcase: igt/kms_blend
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Tested-by: Sharma, Ankitprasad R ankitprasad.r.sha...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 10 ++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_drv.h| 7 +
drivers/gpu/drm/i915/intel_sprite.c | 61 -
5 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e4d2b9f..e3a94a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1898,12 +1898,22 @@ void i915_driver_lastclose(struct drm_device * dev)
{
drm_i915_private_t *dev_priv = dev-dev_private;
+ struct intel_plane *plane;
/* On gen6+ we refuse to init without kms enabled, but then the drm core
* goes right around and calls lastclose. Check for this and don't clean
* up anything. */
if (!dev_priv)
return;
+ if (dev_priv-blend_property) {
+ list_for_each_entry(plane, dev-mode_config.plane_list,
base.head) {
+ plane-blend_param.value = BIT(DRM_BLEND_SRC_COLOR);
+ drm_object_property_set_value(plane-base.base,
+ dev_priv-blend_property,
+ plane-blend_param.value);
+ }
+ }
+
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
intel_fbdev_restore_mode(dev);
vga_switcheroo_process_delayed_switch();
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe90..0d1be70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1607,6 +1607,7 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ struct drm_property *blend_property;
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6174fda..bfcfe72 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3774,6 +3774,8 @@ enum punit_power_well {
#define SPRITE_INT_GAMMA_ENABLE (113)
#define SPRITE_TILED (110)
#define SPRITE_DEST_KEY (12)
+#define SPRITE_CONSTANT_ALPHA_ENABLE (131)
+#define SPRITE_CONSTANT_ALPHA_MASK (0xFF)
#define _SPRA_LINOFF 0x70284
#define _SPRA_STRIDE 0x70288
#define _SPRA_POS 0x7028c
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f4e0615..039a800 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -409,6 +409,13 @@ struct intel_plane {
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y;
uint32_t src_w, src_h;
+ union {
+ uint64_t value;
+ struct {
+ unsigned int type;
+ unsigned int factor;
+ } details;
+ } blend_param;
/* Since we need to change the watermarks before/after
* enabling/disabling the planes, we need to store the parameters here
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 9436f18..9ec84bb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -51,11 +51,15 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
int pipe = intel_plane-pipe;
int plane = intel_plane-plane;
u32 sprctl;
+ u32 sprconstalpha;
+ unsigned int blend_type, blend_factor;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
sprctl = I915_READ(SPCNTR(pipe, plane));
+ sprconstalpha = SPCONSTALPHA(pipe, plane);
+
/* Mask out pixel format bits in case we change it */
sprctl = ~SP_PIXFORMAT_MASK;
sprctl = ~SP_YUV_BYTE_ORDER_MASK;
@@ -104,6 +108,23 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
break;
[Intel-gfx] [PATCH 3/3] drm/i915: Use the actual cursor width for WM computation
Now that we can use different cursor sizes, we can't hardcode 64 pixels as the cursor width anymore. Cc: Sagar Kamble sagar.a.kam...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index faa059a..0ed2a7b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2120,7 +2120,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, p-pri.bytes_per_pixel = crtc-fb-bits_per_pixel / 8; p-cur.bytes_per_pixel = 4; p-pri.horiz_pixels = intel_crtc-config.pipe_src_w; - p-cur.horiz_pixels = 64; + p-cur.horiz_pixels = intel_crtc-cursor_width; /* TODO: for now, assume primary and cursor planes are always enabled. */ p-pri.enabled = true; p-cur.enabled = true; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/4] drm: Added plane alpha and color blending property
From: Sagar Kamble sagar.a.kam...@intel.com
This patch creates a generic blending bitmask property modeled after
glBlendFunc. Drivers may support subset of these values.
v2: Removing blend properties that are not applicable
[Damien's Review Comments].
Adding DRM_MODE_PROP_32BIT_PAIR flag to blend property.
Cc: airl...@linux.ie
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
drivers/gpu/drm/drm_crtc.c | 27 +++
include/drm/drm_crtc.h | 19 +++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d0d03ec..a1f254e 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4157,3 +4157,30 @@ void drm_mode_config_cleanup(struct drm_device *dev)
idr_destroy(dev-mode_config.crtc_idr);
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
+
+struct drm_property *drm_mode_create_blend_property(struct drm_device *dev,
+ unsigned int supported_factors)
+{
+ static const struct drm_prop_enum_list props[] = {
+ { DRM_BLEND_ZERO, zero },
+ { DRM_BLEND_ONE,one },
+ { DRM_BLEND_SRC_COLOR, src-color },
+ { DRM_BLEND_ONE_MINUS_SRC_COLOR,one-minus-src-color },
+ { DRM_BLEND_DST_COLOR, dst-color },
+ { DRM_BLEND_ONE_MINUS_DST_COLOR,one-minus-dst-color },
+ { DRM_BLEND_SRC_ALPHA, src-alpha },
+ { DRM_BLEND_ONE_MINUS_SRC_ALPHA,one-minus-src-alpha },
+ { DRM_BLEND_DST_ALPHA, dst-alpha },
+ { DRM_BLEND_ONE_MINUS_DST_ALPHA,one-minus-dst-alpha },
+ { DRM_BLEND_CONSTANT_COLOR, constant-color },
+ { DRM_BLEND_ONE_MINUS_CONSTANT_COLOR,
one-minus-constant-color },
+ { DRM_BLEND_CONSTANT_ALPHA, constant-alpha },
+ { DRM_BLEND_ONE_MINUS_CONSTANT_ALPHA,
one-minus-constant-alpha },
+ { DRM_BLEND_SRC_ALPHA_SATURATE, alpha-saturate }
+ };
+
+ return drm_property_create_bitmask(dev, DRM_MODE_PROP_32BIT_PAIR,
blend,
+ props, ARRAY_SIZE(props),
+ supported_factors);
+}
+EXPORT_SYMBOL(drm_mode_create_blend_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 784a568..a9fbfec 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -65,6 +65,23 @@ struct drm_object_properties {
uint64_t values[DRM_OBJECT_MAX_PROPERTY];
};
+/* Blending property bits */
+#define DRM_BLEND_ZERO 0
+#define DRM_BLEND_ONE 1
+#define DRM_BLEND_SRC_COLOR2
+#define DRM_BLEND_ONE_MINUS_SRC_COLOR 3
+#define DRM_BLEND_DST_COLOR4
+#define DRM_BLEND_ONE_MINUS_DST_COLOR 5
+#define DRM_BLEND_SRC_ALPHA6
+#define DRM_BLEND_ONE_MINUS_SRC_ALPHA 7
+#define DRM_BLEND_DST_ALPHA8
+#define DRM_BLEND_ONE_MINUS_DST_ALPHA 9
+#define DRM_BLEND_CONSTANT_COLOR 10
+#define DRM_BLEND_ONE_MINUS_CONSTANT_COLOR 11
+#define DRM_BLEND_CONSTANT_ALPHA 12
+#define DRM_BLEND_ONE_MINUS_CONSTANT_ALPHA 13
+#define DRM_BLEND_SRC_ALPHA_SATURATE 14
+
/*
* Note on terminology: here, for brevity and convenience, we refer to
connector
* control chips as 'CRTCs'. They can control any type of connector, VGA,
LVDS,
@@ -1179,6 +1196,8 @@ extern int drm_format_plane_cpp(uint32_t format, int
plane);
extern int drm_format_horz_chroma_subsampling(uint32_t format);
extern int drm_format_vert_chroma_subsampling(uint32_t format);
extern const char *drm_get_format_name(uint32_t format);
+extern struct drm_property *drm_mode_create_blend_property(struct drm_device
*dev,
+ unsigned int supported_factors);
/* Helpers */
static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
--
1.8.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/4] drm: Adding new flag to restrict bitmask drm properties as 32 bit type and 32 bit value pair
From: Sagar Kamble sagar.a.kam...@intel.com
With this patch new flag DRM_MODE_PROP_32BIT_PAIR is added that will help make
use
of 64 bit value of bitmask property as two 32 bit values.
Cc: airl...@linux.ie
Cc: dri-de...@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
drivers/gpu/drm/drm_crtc.c | 22 --
include/uapi/drm/drm_mode.h | 3 +++
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4e43fc2..d0d03ec 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2993,10 +2993,13 @@ int drm_property_add_enum(struct drm_property
*property, int index,
/*
* Bitmask enum properties have the additional constraint of values
-* from 0 to 63
+* from 0 to 63. For properties with 32BIT_PAIR Flag set this constraint
+* range is 0 to 31.
*/
- if ((property-flags DRM_MODE_PROP_BITMASK) (value 63))
- return -EINVAL;
+ if (property-flags DRM_MODE_PROP_BITMASK)
+ if (((property-flags DRM_MODE_PROP_32BIT_PAIR) (value
31)) ||
+ (value 63))
+ return -EINVAL;
if (!list_empty(property-enum_blob_list)) {
list_for_each_entry(prop_enum, property-enum_blob_list, head)
{
@@ -3305,9 +3308,16 @@ static bool drm_property_change_is_valid(struct
drm_property *property,
} else if (property-flags DRM_MODE_PROP_BITMASK) {
int i;
uint64_t valid_mask = 0;
- for (i = 0; i property-num_values; i++)
- valid_mask |= (1ULL property-values[i]);
- return !(value ~valid_mask);
+ uint32_t valid_32bit_mask = 0;
+ if (property-flags DRM_MODE_PROP_32BIT_PAIR) {
+ for (i = 0; i property-num_values; i++)
+ valid_32bit_mask |= (1UL
property-values[i]);
+ return !((value 0x) ~valid_32bit_mask);
+ } else {
+ for (i = 0; i property-num_values; i++)
+ valid_mask |= (1ULL property-values[i]);
+ return !(value ~valid_mask);
+ }
} else if (property-flags DRM_MODE_PROP_BLOB) {
/* Only the driver knows */
return true;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index f104c26..5e3a7d9 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -250,6 +250,9 @@ struct drm_mode_get_connector {
#define DRM_MODE_PROP_ENUM (13) /* enumerated type with text strings */
#define DRM_MODE_PROP_BLOB (14)
#define DRM_MODE_PROP_BITMASK (15) /* bitmask of enumerated types */
+#define DRM_MODE_PROP_32BIT_PAIR (16) /* 32 bit bitmask of enumerated types
+* and 32 bit of value of the type */
+
struct drm_mode_property_enum {
__u64 value;
--
1.8.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 02:54:56PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? It doesn't currently? a property on the cursor plane with the list of supported formats in the brave new full drm_plane world seems like a good option to me. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/4] Documentation: drm: describing plane alpha and color blending property
From: Sagar Kamble sagar.a.kam...@intel.com
v2: Added description for src-color and constant-alpha property.
Cc: Rob Landley rob at landley.net
Cc: Dave Airlie airlied at redhat.com
Cc: Daniel Vetter daniel.vetter at ffwll.ch
Cc: Laurent Pinchart laurent.pinchart+renesas at ideasonboard.com
Cc: David Herrmann dh.herrmann at gmail.com
Cc: Alex Deucher alexander.deucher at amd.com
Cc: Ville Syrjälä ville.syrjala at linux.intel.com
Cc: Sagar Kamble sagar.a.kamble at intel.com
Cc: Purushothaman, Vijay A vijay.a.purushothaman at intel.com
Cc: linux-doc at vger.kernel.org
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
Documentation/DocBook/drm.tmpl | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 104402a..77a45fb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2253,6 +2253,14 @@ void intel_crt_init(struct drm_device *dev)
enumerated bits defined by the property./para/listitem
/varlistentry
varlistentry
+ termDRM_MODE_PROP_32BIT_PAIR/term
+ listitemparaThis flag restricts Bitmask properties restricts all
+ enumerated values to the 0..31 range.
+During get operation instance values combine one or more of the
+enumerated bits defined by the property. During get user can
specify
+ {type, value} pair./para/listitem
+/varlistentry
+varlistentry
termDRM_MODE_PROP_BLOB/term
listitemparaBlob properties store a binary blob without any
format
restriction. The binary blobs are created as KMS standalone
objects,
@@ -2336,7 +2344,7 @@ void intel_crt_init(struct drm_device *dev)
/tr
tr
td rowspan=19 valign=top DRM/td
- td rowspan=2 valign=top Generic/td
+ td rowspan=3 valign=top Generic/td
td valign=top “EDID”/td
td valign=top BLOB | IMMUTABLE/td
td valign=top 0/td
@@ -2351,6 +2359,19 @@ void intel_crt_init(struct drm_device *dev)
td valign=top Contains DPMS operation mode value./td
/tr
tr
+ td valign=top “blend”/td
+ td valign=top BITMASK | 32BIT_PAIR/td
+ td valign=top { {0, zero}, {1, one}, {2, src-color}, {3,
one-minus-src-color}
+ , {4, dst-color}, {5, one-minus-dst-color}, {6, src-alpha}, {7,
one-minus-src-alpha}, {8, dst-alpha}
+ , {9, one-minus-dst-alpha}, {10, constant-color}, {11,
one-minus-constant-color}, {12, constant-alpha}
+ , {13, one-minus-constant-alpha}, {14, alpha-saturate} }/td
+ td valign=top Plane/td
+ td valign=top Contains plane alpha/color blending operation values.
These are modeled after glBlendFunc API
+ in OpenGL. Currently only src-color and constant-alpha are
supported. parasrc-color will consider supplied fb
+ with plane's pixel format as input without any additional color/alpha
changes./paraparaconstant-alpha will apply constant
+ transparency to all pixels in addition to source color./para/td
+ /tr
+ tr
td rowspan=2 valign=top DVI-I/td
td valign=top “subconnector”/td
td valign=top ENUM/td
--
1.8.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 02:59:18PM +, Damien Lespiau wrote: On Tue, Mar 25, 2014 at 02:54:56PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? It doesn't currently? a property on the cursor plane with the list of supported formats in the brave new full drm_plane world seems like a good option to me. Userspace currently uses this method to determine the largest cursor supported by the driver. That userspace is inept at resize the cursor bo as required is a probably that won't be solved by simply exposing it later. -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 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, 2014-03-25 at 14:59 +, Damien Lespiau wrote: On Tue, Mar 25, 2014 at 02:54:56PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? It doesn't currently? a property on the cursor plane with the list of supported formats in the brave new full drm_plane world seems like a good option to me. How do we handle cursor size cap that was added with http://lists.freedesktop.org/archives/dri-devel/2014-February/053770.html. Will change i-g-t kms_cursor_crc accordingly as well since it depends on this cap. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, 2014-03-25 at 14:49 +, Damien Lespiau wrote:
Those fields are supposed to be a good default value for the cursor
size, intended for the case where the hardware doesn't support 64x64
cursors, for use with a hw agnostic DDX driver for instance.
We're fine with 64x64 cursors though and don't need to set those fields
(DRM core will return 64 is we don't). If we declare 256x256, that
generic driver will use a big buffer for the cursor, without any good
reason.
Cc: Sagar Kamble sagar.a.kam...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 80dd1c2..7a47657 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10557,8 +10557,6 @@ static void intel_crtc_init(struct drm_device *dev,
int pipe)
intel_crtc-max_cursor_width = CURSOR_WIDTH;
intel_crtc-max_cursor_height = CURSOR_HEIGHT;
}
- dev-mode_config.cursor_width = intel_crtc-max_cursor_width;
- dev-mode_config.cursor_height = intel_crtc-max_cursor_height;
I thought drm_getcap should return the max cursor size, using these two
fields.
--Imre
drm_mode_crtc_set_gamma_size(intel_crtc-base, 256);
for (i = 0; i 256; i++) {
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
Those fields are supposed to be a good default value for the cursor
size, intended for the case where the hardware doesn't support 64x64
cursors, for use with a hw agnostic DDX driver for instance.
We're fine with 64x64 cursors though and don't need to set those fields
(DRM core will return 64 is we don't). If we declare 256x256, that
generic driver will use a big buffer for the cursor, without any good
reason.
Cc: Sagar Kamble sagar.a.kam...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 80dd1c2..7a47657 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10557,8 +10557,6 @@ static void intel_crtc_init(struct drm_device *dev, int
pipe)
intel_crtc-max_cursor_width = CURSOR_WIDTH;
intel_crtc-max_cursor_height = CURSOR_HEIGHT;
}
- dev-mode_config.cursor_width = intel_crtc-max_cursor_width;
- dev-mode_config.cursor_height = intel_crtc-max_cursor_height;
drm_mode_crtc_set_gamma_size(intel_crtc-base, 256);
for (i = 0; i 256; i++) {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix up semaphore_waits_for
On Tue, Mar 25, 2014 at 10:42:09AM +0100, Daniel Vetter wrote: On Mon, Mar 24, 2014 at 03:37:12PM -0700, Ben Widawsky wrote: On Sat, Mar 22, 2014 at 06:52:25PM +0100, Daniel Vetter wrote: On Fri, Mar 21, 2014 at 07:33:59PM +0200, Mika Kuoppala wrote: Hi, Daniel Vetter daniel.vet...@ffwll.ch writes: There's an entire pile of issues in here: - Use the main RING_HEAD register, not ACTHD. ACTHD points at the gtt offset of the batch buffer when a batch is executed. Semaphores are always emitted to the main ring, so we always want to look at that. The ipehr check should make sure that we are at the ring and acthd should not be at batch. Regardless I agree that RING_HEAD is much more safer. When I have tried to get bottom of the snb reset hang, I have noticed that after reset the acthd is sometimes 0x0c even tho head is 0x00, on snb. Hm, should we maybe mask out the lower bits, too? If you can confirm this, can you please add a follow-up patch? - Mask the obtained HEAD pointer with the actual ring size, which is much smaller. Together with the above issue this resulted us in trying to dereference a pointer way outside of the ring mmio mapping. The resulting invalid access in interrupt context (hangcheck is executed from timers) lead to a full blown kernel panic. The fbcon panic handler then tried to frob our driver harder, resulting in a full machine hang at least on my snb here where I've stumbled over this. - Handle ring wrapping correctly and be a bit more explicit about how many dwords we're scanning. We probably should also scan more than just 4 ... ipehr dont update on MI_NOOPS and the head should point to the extra MI_NOOP after semaphore sequence. So 4 should be enough. Perhaps revisit this when bdw gains semaphores. Yeah, Chris also mentioned that the HEAD should point at one dword past the end of the ring, so should even work when there are no MI_NOOPs. In any case this code is more robust in case hw engineers suddenly change the behaviour. - Space out some of teh computations for readability. This prevents hard-hangs on my snb here. Verdict from QA is still pending, but the symptoms match. Cc: Mika Kuoppala mika.kuopp...@intel.com Cc: Ben Widawsky b...@bwidawsk.net Cc: Chris Wilson ch...@chris-wilson.co.uk Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74100 The hard hangs are not so frequent with this patch but they are still there. This patch should take care of the oops we have been seeing, but there is still something else to be found until #74100 can be marked as fixed. Very often after reset, when igt has pushed the quietence batches into rings, blitter and video ring heads gets moved properly but all updates to hardware status page and to the sync registers are missing. And result is hang by hangcheck. Repeat enough times and it is a hard hang. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Please remove the blowup comment and also update the bugzilla tag. Yeah, QA also says that it doesn't fix the hard hangs, only seems to reduce them a bit on certain setups. I've updated the commit message. btw are you testing with FBDEV=n? The lack of a fbcon panic handler should greatly increase the chances that the last few message get correctly transmitted through other means like netconsole. Patches 1-2 and the followup one are Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com Thanks for the review, patches merged. -Daniel Patch 2 was trivial to implement for gen8. This functionality is a lot less trivial. I volunteered to do patch 2, are you going to port this to gen8? If you fill out the bdw pieces for patch 23 the only thing you need to change here is to adjsut the number of dwords we walk backwards to make sure that the bdw semaphore cmd still fits. Or at least that's been my idea behind all this, maybe I've overlooked something. -Daniel I don't think it's quite that easy, but I took it as a, yes. Which one is patch 3? -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix up semaphore_waits_for
On Tue, Mar 25, 2014 at 3:54 PM, Ben Widawsky b...@bwidawsk.net wrote: I don't think it's quite that easy, but I took it as a, yes. Which one is patch 3? drm/i915: make semaphore signaller detection more robust - it was a follow-up patch in this thread, hence no 3/3. If the relevant bits on bdw have moved out of the the cmd dword I guess we need to keep a cache of the relevant dword and supply it to the function. Was too lazy to check that though. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 22/48] drm/i915: Use drm_mm for PPGTT PDEs
On Tue, Mar 25, 2014 at 01:41:25PM +, Chris Wilson wrote:
On Mon, Mar 24, 2014 at 01:02:57PM -0700, Ben Widawsky wrote:
On Mon, Mar 24, 2014 at 07:45:56PM +, Chris Wilson wrote:
On Mon, Mar 24, 2014 at 12:36:23PM -0700, Ben Widawsky wrote:
On Thu, Mar 20, 2014 at 11:10:13AM +, Chris Wilson wrote:
On Fri, Dec 06, 2013 at 02:11:55PM -0800, Ben Widawsky wrote:
static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
{
+#define GEN6_PD_ALIGN (PAGE_SIZE * 16)
+#define GEN6_PD_SIZE (GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE)
struct drm_device *dev = ppgtt-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- unsigned first_pd_entry_in_global_pt;
- int i;
- int ret = -ENOMEM;
+ int i, ret;
- /* ppgtt PDEs reside in the global gtt pagetable, which has
512*1024
-* entries. For aliasing ppgtt support we just steal them at
the end for
-* now. */
- first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt);
+ /* PPGTT PDEs reside in the GGTT and consists of 512 entries.
The
+* allocator works in address space sizes, so it's multiplied
by page
+* size. We allocate at the top of the GTT to avoid
fragmentation.
+*/
+ BUG_ON(!drm_mm_initialized(dev_priv-gtt.base.mm));
+ ret =
drm_mm_insert_node_in_range_generic(dev_priv-gtt.base.mm,
+ ppgtt-node,
GEN6_PD_SIZE,
+ GEN6_PD_ALIGN, 0,
+ 0,
dev_priv-gtt.base.total,
+
DRM_MM_SEARCH_DEFAULT);
This could use the simpler drm_mm_insert_node_generic().
-Chris
Not with my [simple] workaround to not use offset 0, which Daniel
reverted. I think he has some hope that we'll actually be able to figure
out why we can't use offset 0 instead of just using the workaround.
You can simply reduce the drm_mm range...
-Chris
Yeah, that's a better solution. Patches welcome?
I thought I had mentioned this earlier, but alas couldn't find it in
email. So here goes,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ee53551..0124e2c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1227,7 +1227,9 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
if (!ret) {
struct drm_i915_private *dev_priv = dev-dev_private;
kref_init(ppgtt-ref);
- drm_mm_init(ppgtt-base.mm, ppgtt-base.start,
+ /* Magic offset because we have no idea what's going on */
+ drm_mm_init(ppgtt-base.mm,
+ ppgtt-base.start + 4096,
Do we have bugzillas and stuff like backtraces, just for the record?
I'd still like to know what's going on here and what exactly blows up.
Iirc you've fixed some of the bogus assumptions in some of the igts ...
-Daniel
ppgtt-base.total);
i915_init_vm(dev_priv, ppgtt-base);
if (INTEL_INFO(dev)-gen 8) {
--
Chris Wilson, Intel Open Source Technology Centre
--
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: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 08:15:54AM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 09:07:00AM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 9:03 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Feels a bit like duct-tape ... Shouldn't we instead stop tearing down ringbuffer structures over suspend/resume? And maybe the same for init with your patch applied. Even if we did, we would still end up with g45 failing to restart the ring at random, so we need some w/a. Or we simply check for wedged first thing in execbuf ... See the first 2 patches ;-) The first is actually essential as we have no other guard against writing into the non-existent ring. I thought about doing that. However, I prefer doing arg validation first i.e. get all (or as much as is feasible) of the EINVAL checks out of the way so that we avoid touching hardware or leaking any information to a malicious client. The problem in this case is where is not actually an invalid arg. Note that we will detect the EIO later before touching hardware (so long as the first two patches are in place). Yeah I've seen the other patches. I think we should try to keep all the ring structures around even when the hw init failed. I've made some feeble attempts a while ago to split the structure init from the hw init stuff, but kinda never fully materialized ... Imo if our set of valid rings semi-randomly changes at runtime even, that's not good. -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] drm/i915: use hrtimer in wait for vblank
On Tuesday 25 March 2014 03:02 PM, Jani Nikula wrote:
On Tue, 25 Mar 2014, Chris Wilson ch...@chris-wilson.co.uk wrote:
On Tue, Mar 25, 2014 at 02:28:22PM +0530, Arun R Murthy wrote:
In wait for vblank use usleep_range, which will use hrtimers instead of
msleep. Using msleep(1~20) there are more chances of sleeping for 20ms.
Using usleep_range uses hrtimers and hence are precise, worst case will
trigger an interrupt at the higher/max timeout.
As per kernel document Documentation/timers/timers-howto.txt sleeping
for 10us to 20ms its recomended to use usleep_range.
Signed-off-by: Arun R Murthy arun.r.mur...@intel.com
Lgtm, I still feel that our use of W=1 is fairly arbitrary and worth
tweaking in future.
With the current code, this is essentially the same as the original
patch. We never have W 20, and thus we always take the usleep_range()
path. So W is definitely worth tweaking if we go with this now.
Nitpick, the macro params should be parenthesized. This will now break
for _wait_for(cond, 10, 2 + 1) and such.
wait_for(COND, TIMEOUT, ATOMIC, MS)
and remove all wait_for_X
function will look like
_wait_for(COND TIMEOUT, ATOMIC, MS)
{
/* loop */
/* check condition */
if (atomic)
cpu_relax()
else
if (ms 20)
msleep
else
usleep_range
}
caller for wait_for will be setting all the parameters and hence no tweaks.
Thanks and Regards,
Arun R Murthy
---
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: use hrtimer in wait for vblank
On Tuesday 25 March 2014 01:00 PM, Chris Wilson wrote: On Tue, Mar 25, 2014 at 11:29:02AM +0530, Arun R Murthy wrote: In wait for vblank use usleep_range, which will use hrtimers instead of msleep. Using msleep(1~20) there are more chances of sleeping for 20ms. Using usleep_range uses hrtimers and hence are precise, worst case will trigger an interrupt at the higher/max timeout. Change-log: On replacing msleep(1) with usleep_range(1000, 2000) we have noticed the time consumed by wait for vblank is ~4ms to ~17ms. Change-Id: I6672e5697b01987a6d069ab06e76d97287b1f7ae Signed-off-by: Arun R Murthy arun.r.mur...@intel.com No. I feel strongly that we do not want more wait_for_X() with strange semantics. http://sweng.the-davies.net/Home/rustys-api-design-manifesto Will revert this additional wait_for_X. Will update the existing _wait_for as per the kernel documentation for timers. Thanks and Regards, Arun R Murthy -- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/6] drm/i915: Remove the enabling of VS_TIMER_DISPATCH bit in MI MODE reg
On Tue, Mar 25, 2014 at 03:11:10PM +0200, Ville Syrjälä wrote: On Tue, Mar 25, 2014 at 06:01:50PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com This patch Removes the VS_TIMER_DISPATCH bit enable in MI MODE reg for platforms Gen6. VS_TIMER_DISPATCH bit enable was earlier required as a part of WA 'WaTimedSingleVertexDispatch', which is now applicable only to platforms Gen7. v2: Enhancing the scope of the patch to full Gen7 (Chris) v3: Modifying the WA condition to the cover the applicable platforms, and adding the WA name in comments. (Ville) Signed-off-by: Akash Goel akash.g...@intel.com Signed-off-by: Sourab Gupta sourab.gu...@intel.com Tested-by: Chris Wilson ch...@chris-wilson.co.uk # ivb, hsw -Chris Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 08:03:28AM +, Chris Wilson wrote:
If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.
This should prevent UXA from getting stuck in a null render loop after a
failed resume.
Reported-by: Jiri Kosina ji...@jikos.cz
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b45163e19f3..22c650490f54 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -991,6 +991,18 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
}
+static bool
+intel_ring_valid(struct intel_ring_buffer *ring)
+{
+ switch (ring-id) {
+ case RCS: return true;
+ case VCS: return HAS_BSD(ring-dev);
+ case BCS: return HAS_BLT(ring-dev);
intel_enable_blt()?
+ case VECS: return HAS_VEBOX(ring-dev);
+ default: return false;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1041,7 +1053,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if (!intel_ring_initialized(ring)) {
DRM_DEBUG(execbuf with invalid ring: %d\n,
(int)(args-flags I915_EXEC_RING_MASK));
- return -EINVAL;
+ return intel_ring_valid(ring) ? -EIO : -EINVAL;
}
mode = args-flags I915_EXEC_CONSTANTS_MASK;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 03:09:16PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:59:18PM +, Damien Lespiau wrote: On Tue, Mar 25, 2014 at 02:54:56PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? It doesn't currently? a property on the cursor plane with the list of supported formats in the brave new full drm_plane world seems like a good option to me. Userspace currently uses this method to determine the largest cursor supported by the driver. That userspace is inept at resize the cursor bo as required is a probably that won't be solved by simply exposing it later. That doesn't seem to be the intention of the original patch? http://lists.freedesktop.org/archives/dri-devel/2014-February/053770.html Are you saying the Intel DDX currently derives a different meaning to the intented behaviour? in which case it can still be changed to not do that? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Remove max_cursor_{width, height} from the crtc
We don't need them anymore as they were used to initialize DRM's
mode_config.
As a side note, it was a bit strange to put them on the CRTC object,
they are global values, not specific to a CRTC.
As another side note, those values were only used in that one function,
so we didn't need to store them elsewhere.
Cc: Sagar Kamble sagar.a.kam...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 8
drivers/gpu/drm/i915/intel_drv.h | 7 ---
2 files changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7a47657..3d1ecd0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10550,14 +10550,6 @@ static void intel_crtc_init(struct drm_device *dev,
int pipe)
drm_crtc_init(dev, intel_crtc-base, intel_crtc_funcs);
- if (IS_GEN2(dev)) {
- intel_crtc-max_cursor_width = GEN2_CURSOR_WIDTH;
- intel_crtc-max_cursor_height = GEN2_CURSOR_HEIGHT;
- } else {
- intel_crtc-max_cursor_width = CURSOR_WIDTH;
- intel_crtc-max_cursor_height = CURSOR_HEIGHT;
- }
-
drm_mode_crtc_set_gamma_size(intel_crtc-base, 256);
for (i = 0; i 256; i++) {
intel_crtc-lut_r[i] = i;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fa99104..60ffad3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -78,12 +78,6 @@
#define MAX_OUTPUTS 6
/* maximum connectors per crtcs in the mode set */
-/* Maximum cursor sizes */
-#define GEN2_CURSOR_WIDTH 64
-#define GEN2_CURSOR_HEIGHT 64
-#define CURSOR_WIDTH 256
-#define CURSOR_HEIGHT 256
-
#define INTEL_I2C_BUS_DVO 1
#define INTEL_I2C_BUS_SDVO 2
@@ -373,7 +367,6 @@ struct intel_crtc {
uint32_t cursor_addr;
int16_t cursor_x, cursor_y;
int16_t cursor_width, cursor_height;
- int16_t max_cursor_width, max_cursor_height;
bool cursor_visible;
struct intel_plane_config plane_config;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 08:24:00AM -0700, Ben Widawsky wrote:
On Tue, Mar 25, 2014 at 08:03:28AM +, Chris Wilson wrote:
If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.
This should prevent UXA from getting stuck in a null render loop after a
failed resume.
Reported-by: Jiri Kosina ji...@jikos.cz
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b45163e19f3..22c650490f54 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -991,6 +991,18 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev,
return 0;
}
+static bool
+intel_ring_valid(struct intel_ring_buffer *ring)
+{
+ switch (ring-id) {
+ case RCS: return true;
+ case VCS: return HAS_BSD(ring-dev);
+ case BCS: return HAS_BLT(ring-dev);
intel_enable_blt()?
But not exported, and below my level of caring ;-)
The cases were intel_enable_blt() != HAS_BLT() are those where the hw
would hang anyway.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 04:36:05PM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 08:15:54AM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 09:07:00AM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 9:03 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Feels a bit like duct-tape ... Shouldn't we instead stop tearing down ringbuffer structures over suspend/resume? And maybe the same for init with your patch applied. Even if we did, we would still end up with g45 failing to restart the ring at random, so we need some w/a. Or we simply check for wedged first thing in execbuf ... See the first 2 patches ;-) The first is actually essential as we have no other guard against writing into the non-existent ring. I thought about doing that. However, I prefer doing arg validation first i.e. get all (or as much as is feasible) of the EINVAL checks out of the way so that we avoid touching hardware or leaking any information to a malicious client. The problem in this case is where is not actually an invalid arg. Note that we will detect the EIO later before touching hardware (so long as the first two patches are in place). Yeah I've seen the other patches. I think we should try to keep all the ring structures around even when the hw init failed. I've made some feeble attempts a while ago to split the structure init from the hw init stuff, but kinda never fully materialized ... Imo if our set of valid rings semi-randomly changes at runtime even, that's not good. Agreed, but sadly we can't trust hardware to always work, and we need something to prevent explosions. I quite like the idea of marking the GPU wedged if hw init fails so that we lose acceleration but keep modesetting around. -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 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 04:05:29PM +, Damien Lespiau wrote: On Tue, Mar 25, 2014 at 03:09:16PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:59:18PM +, Damien Lespiau wrote: On Tue, Mar 25, 2014 at 02:54:56PM +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 02:49:30PM +, Damien Lespiau wrote: Those fields are supposed to be a good default value for the cursor size, intended for the case where the hardware doesn't support 64x64 cursors, for use with a hw agnostic DDX driver for instance. We're fine with 64x64 cursors though and don't need to set those fields (DRM core will return 64 is we don't). If we declare 256x256, that generic driver will use a big buffer for the cursor, without any good reason. How does userspace now know that 256x256 cursors are support for HiDPI? It doesn't currently? a property on the cursor plane with the list of supported formats in the brave new full drm_plane world seems like a good option to me. Userspace currently uses this method to determine the largest cursor supported by the driver. That userspace is inept at resize the cursor bo as required is a probably that won't be solved by simply exposing it later. That doesn't seem to be the intention of the original patch? http://lists.freedesktop.org/archives/dri-devel/2014-February/053770.html For the record, 16:30 agd5f ickle, our GPUs don't have selectable cursor sizes 16:31 agd5f so on the newer ones, xf86-video-modesetting, etc. would allocate a 64x64 cursor and it would look squashed and funky since the hw expects 128x128 Which means I was confused when I thought part of the reasoning was indeed HiDPI support. (I'm still seem to remember that was part of the argument for large cursors anyway.) Are you saying the Intel DDX currently derives a different meaning to the intented behaviour? in which case it can still be changed to not do that? I still disagree though. This provides all the information I need to support variable sized cursors and we can use large cursors today. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Allow full PPGTT with param override
On Tue, Mar 25, 2014 at 10:46:12AM +0100, Daniel Vetter wrote: On Mon, Mar 24, 2014 at 06:06:00PM -0700, Ben Widawsky wrote: When PPGTT was disabled by default, the patch also prevented the user from overriding this behavior via module parameter. Being able to test this on arbitrary kernels is extremely beneficial to track down the remaining bugs. The patch that prevented this was: commit 93a25a9e2d67765c3092bfaac9b855d95e39df97 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Mar 6 09:40:43 2014 +0100 drm/i915: Disable full ppgtt by default By default PPGTT is set to -1. 0 means off, 1 means aliasing only, 2 means full, all other values are reserved. Signed-off-by: Ben Widawsky b...@bwidawsk.net My apologies for breaking this a bit harder than intended, and thanks for fixing it up. Patch merged to dinq. -Daniel No harm, no foul. FWIW QA had been reported 0 PPGTT regressions for the last week or so. Score one for their consistency. --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index bbc9420..9068628 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -50,7 +50,7 @@ bool intel_enable_ppgtt(struct drm_device *dev, bool full) /* Full ppgtt disabled by default for now due to issues. */ if (full) - return false; /* HAS_PPGTT(dev) */ + return HAS_PPGTT(dev) (i915.enable_ppgtt == 2); else return HAS_ALIASING_PPGTT(dev); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] README: document quirks for regenerating gtk-doc
It sucks a bit atm :( Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- README | 11 +++ 1 file changed, 11 insertions(+) diff --git a/README b/README index 66462c8a9bda..7ae9bb2e2609 100644 --- a/README +++ b/README @@ -115,6 +115,17 @@ docs/ reference documenation in docs/reference/ You need to have the gtk doc tools installed to generate this API documentation. + Note that the currrent gtk-docs integration sucks a bit wrt regenerating + the html files. You need at least + + $ make clean -C docs make -C docs + + to regenerate them on any change. If you've added/changed/removed a + symbol or anything else that changes the overall structure or indexes, + you need a full rebuild: + + $ git clean -dfx ./autogen.sh --enable-gtk-doc make -C docs + DEPENDENCIES This is a non-exchaustive list of package dependencies required for building everything: -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Allow full PPGTT with param override
On Tue, Mar 25, 2014 at 5:42 PM, Ben Widawsky b...@bwidawsk.net wrote: My apologies for breaking this a bit harder than intended, and thanks for fixing it up. Patch merged to dinq. -Daniel No harm, no foul. FWIW QA had been reported 0 PPGTT regressions for the last week or so. Score one for their consistency. Well, let's see whether they start to see all the issues again then ... I guess I could say this was all a sneaky trick from me to have a proper baseline from our QA people ;-) -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: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 5:29 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: Yeah I've seen the other patches. I think we should try to keep all the ring structures around even when the hw init failed. I've made some feeble attempts a while ago to split the structure init from the hw init stuff, but kinda never fully materialized ... Imo if our set of valid rings semi-randomly changes at runtime even, that's not good. Agreed, but sadly we can't trust hardware to always work, and we need something to prevent explosions. I quite like the idea of marking the GPU wedged if hw init fails so that we lose acceleration but keep modesetting around. Yeah, I agree that the other two patches are neat indeed, it's this one here where the shiny starts to come off a bit ;-) tbh I'd prefer a simply if (terminally_wedged) return -EIO; here before the ring checks, maybe with a comment stating why we need to have this order. That, or fix the mess called ring init code ... -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/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 04:38:24PM +, Chris Wilson wrote: For the record, 16:30 agd5f ickle, our GPUs don't have selectable cursor sizes 16:31 agd5f so on the newer ones, xf86-video-modesetting, etc. would allocate a 64x64 cursor and it would look squashed and funky since the hw expects 128x128 Which means I was confused when I thought part of the reasoning was indeed HiDPI support. (I'm still seem to remember that was part of the argument for large cursors anyway.) Are you saying the Intel DDX currently derives a different meaning to the intented behaviour? in which case it can still be changed to not do that? I still disagree though. This provides all the information I need to support variable sized cursors and we can use large cursors today. I'd love the game to be about defining clear semantics more than by interpreting that value this way, I got what I always wanted :) We can resolve that today with MAX_CURSOR_WIDTH, MAX_CURSOR_HEIGHT caps as well (if you're alluding at the fact that drm_planes may still be a few decades away). We'll still need to expose the full list of supported cursor sizes for compositors at some point or another, my preferred way would be with a property in the exposed cursor drm_plane. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] lib: add igt_get_stop_rings and igt_set_stop_rings
Multiple tests are introducing hangs by fidding with i915_ring_stop
debugfs entry.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
lib/igt_debugfs.c | 107 +
lib/igt_debugfs.h | 28 ++
2 files changed, 135 insertions(+)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index e04f8c5..c4cf50d 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -28,9 +28,11 @@
#include errno.h
#include stdio.h
#include stdlib.h
+#include limits.h
#include string.h
#include fcntl.h
#include unistd.h
+#include i915_drm.h
#include drmtest.h
#include igt_display.h
@@ -615,3 +617,108 @@ int igt_open_forcewake_handle(void)
{
return igt_debugfs_open(i915_forcewake_user, O_WRONLY);
}
+
+/**
+ * igt_to_stop_ring_flag:
+ * @ring: ring to get flag for, in drm I915_EXEC_* namespace
+ *
+ * This converts ring specified (drm namespace) to a ring flag
+ * to be used with igt_get_stop_rings() and igt_set_stop_rings().
+ *
+ * Returns:
+ * Ring flag for the give ring
+ */
+enum stop_ring_flags igt_to_stop_ring_flag(int ring) {
+ if (ring == I915_EXEC_DEFAULT)
+ return STOP_RING_RENDER;
+
+ igt_assert(ring ((ring ~I915_EXEC_RING_MASK) == 0));
+ return 1 (ring - 1);
+}
+
+static void stop_rings_write(uint32_t mask)
+{
+ int fd;
+ char buf[80];
+
+ igt_assert(snprintf(buf, sizeof(buf), 0x%08x, mask) == 10);
+ fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(write(fd, buf, strlen(buf)) == strlen(buf));
+ close(fd);
+}
+
+/**
+ * igt_get_stop_rings:
+ *
+ * Read current ring flags from 'i915_ring_stop' debugfs entry.
+ *
+ * Returns:
+ * Current ring flags
+ */
+enum stop_ring_flags igt_get_stop_rings(void)
+{
+ int fd;
+ char buf[80];
+ int l;
+ unsigned long long ring_mask;
+
+ fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
+ igt_assert(fd = 0);
+ l = read(fd, buf, sizeof(buf));
+ igt_assert(l 0);
+ igt_assert(l sizeof(buf));
+
+ buf[l] = '\0';
+
+ close(fd);
+
+ errno = 0;
+ ring_mask = strtoull(buf, NULL, 0);
+ igt_assert(errno == 0);
+ return ring_mask;
+}
+
+/**
+ * igt_set_stop_rings:
+ * @flags: Ring flags to write
+ *
+ * This writes @flags to 'i915_ring_stop' debugfs entry. Driver will
+ * prevent cpu from writing tail pointer for ring @flags specify. Note
+ * that the ring is not stopped right away. Instead any further command
+ * emissions won't be executed after the flag is set.
+ *
+ * This is the least invasive way to make the gpu stuck. Hence you must
+ * set this after a batch submission with it's own invalid or endless
+ * looping instructions. In this case it is merely to give a log
+ * notification that this was simulated hang, as the batch would have
+ * caused hang in any case. Or if you use valid or noop batch and want
+ * to hang the ring (gpu), you must set this flag before submitting the
+ * batch.
+ *
+ * Driver checks periodically if ring is making any progress, and if
+ * not, it will declare the ring to be hung and will reset the GPU.
+ * After reset, the driver will clear flags in 'i915_ring_stop'
+ *
+ * Note: Always when hanging the gpu, use igt_set_stop_rings() to
+ * notify the driver. Driver tunes down gpu hang log messaging
+ * based on these flags and thus prevents false positives on logs.
+ *
+ */
+void igt_set_stop_rings(enum stop_ring_flags flags)
+{
+ enum stop_ring_flags current;
+
+ igt_assert((flags ~STOP_RING_ALL) == 0);
+
+ current = igt_get_stop_rings();
+ igt_assert_f(current == 0,
+previous i915_ring_stop is still 0x%x\n, current);
+
+ stop_rings_write(flags);
+ current = igt_get_stop_rings();
+ if (current != flags)
+ igt_warn(i915_ring_stop readback mismatch 0x%x vs 0x%x\n,
+flags, current);
+}
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 3312a8b..72f4999 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -143,4 +143,32 @@ void igt_enable_prefault(void);
int igt_open_forcewake_handle(void);
+/**
+ * stop_ring_flags:
+ *
+ * @STOP_RING_NONE: Can be used to clear the pending stop (warning: hang might
+ * be declared already). Returned by igt_get_stop_rings() if there is
+ * no currently stopped rings.
+ * @STOP_RING_RENDER: Render ring
+ * @STOP_RING_BSD: Video encoding/decoding ring
+ * @STOP_RING_BLT: Blitter ring
+ * @STOP_RING_VEBOX: Video enchanment ring
+ * @STOP_RING_ALL: All rings
+ *
+ * Enumeration of all supported flags for igt_set_stop_rings().
+ *
+ */
+enum stop_ring_flags {
+ STOP_RING_NONE = 0,
+ STOP_RING_RENDER = (1 0),
+ STOP_RING_BSD = (1 1),
+ STOP_RING_BLT = (1 2),
+ STOP_RING_VEBOX = (1 3),
+ STOP_RING_ALL = 0xff,
+};
+
+enum stop_ring_flags igt_to_stop_ring_flag(int ring);
+void igt_set_stop_rings(enum
[Intel-gfx] [PATCH 2/2] tests: use lib igt_[get|set]_stop_rings()
on gem_reset_stats, kms_flip and pm_rps.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
tests/gem_reset_stats.c | 30 +-
tests/kms_flip.c| 37 +
tests/pm_rps.c | 35 ++-
3 files changed, 8 insertions(+), 94 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 28679e7..d43e37e 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -236,37 +236,9 @@ static int exec_valid(int fd, int ctx)
return exec_valid_ring(fd, ctx, current_ring-exec);
}
-static void stop_rings(const int mask)
-{
- int fd;
- char buf[80];
-
- igt_assert((mask ~((1 NUM_RINGS) - 1)) == 0);
- igt_assert(snprintf(buf, sizeof(buf), 0x%02x, mask) == 4);
- fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
- igt_assert(fd = 0);
-
- igt_assert(write(fd, buf, 4) == 4);
- close(fd);
-}
-
#define BUFSIZE (4 * 1024)
#define ITEMS (BUFSIZE 2)
-static int ring_to_mask(int ring)
-{
- for (unsigned i = 0; i NUM_RINGS; i++) {
- const struct target_ring *r = rings[i];
-
- if (r-exec == ring)
- return (1 i);
- }
-
- igt_assert(0);
-
- return -1;
-}
-
static int inject_hang_ring(int fd, int ctx, int ring)
{
struct drm_i915_gem_execbuffer2 execbuf;
@@ -356,7 +328,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
free(buf);
- stop_rings(ring_to_mask(ring));
+ igt_set_stop_rings(igt_to_stop_ring_flag(ring));
return exec.handle;
}
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 7ba1656..24ef527 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -44,6 +44,7 @@
#include intel_batchbuffer.h
#include igt_kms.h
#include igt_aux.h
+#include igt_debugfs.h
#define TEST_DPMS (1 0)
#define TEST_WITH_DUMMY_BCS(1 1)
@@ -692,35 +693,14 @@ static void set_y_tiling(struct test_output *o, int
fb_idx)
static void stop_rings(bool stop)
{
- static const char dfs_base[] = /sys/kernel/debug/dri;
- static const char dfs_entry[] = i915_ring_stop;
- static const char stop_data[] = 0xf;
- static const char run_data[] = 0x0;
- char fname[FILENAME_MAX];
- int card_index = drm_get_card();
- int fd;
-
- snprintf(fname, FILENAME_MAX, %s/%i/%s,
-dfs_base, card_index, dfs_entry);
-
- fd = open(fname, O_WRONLY);
- igt_assert(fd = 0);
-
- if (stop)
- igt_assert(write(fd, stop_data, sizeof(stop_data)) ==
sizeof(stop_data));
- else
- igt_assert(write(fd, run_data, sizeof(run_data)) ==
sizeof(run_data));
-
- close(fd);
+ igt_set_stop_rings(stop ? STOP_RING_ALL : STOP_RING_NONE);
}
static void eat_error_state(void)
{
static const char dfs_base[] = /sys/kernel/debug/dri;
static const char dfs_entry_error[] = i915_error_state;
- static const char dfs_entry_stop[] = i915_ring_stop;
static const char data[] = ;
- static char tmp[128];
char fname[FILENAME_MAX];
int card_index = drm_get_card();
int fd;
@@ -739,16 +719,9 @@ static void eat_error_state(void)
/* and check whether stop_rings is not reset, i.e. the hang has indeed
* happened */
- snprintf(fname, FILENAME_MAX, %s/%i/%s,
-dfs_base, card_index, dfs_entry_stop);
-
- fd = open(fname, O_RDONLY);
- igt_assert(fd = 0);
-
- igt_assert(read(fd, tmp, sizeof tmp) 0);
-
- igt_assert_f(atoi(tmp) == 0,
-no gpu hang detected, stop_rings is still %s\n, tmp);
+ igt_assert_f(igt_get_stop_rings() == STOP_RING_NONE,
+no gpu hang detected, stop_rings is still 0x%x\n,
+igt_get_stop_rings());
close(fd);
}
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 157f9e3..66ed5e0 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -306,37 +306,6 @@ static void load_helper_deinit(void)
drm_intel_bufmgr_destroy(lh.bufmgr);
}
-static void stop_rings(void)
-{
- int fd;
- static const char data[] = 0xf;
-
- fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
- igt_assert(fd = 0);
-
- igt_debug(injecting ring stop\n);
- igt_assert(write(fd, data, sizeof(data)) == sizeof(data));
-
- close(fd);
-}
-
-static bool rings_stopped(void)
-{
- int fd;
- static char buf[128];
- unsigned long long val;
-
- fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
- igt_assert(fd = 0);
-
- igt_assert(read(fd, buf, sizeof(buf)) 0);
- close(fd);
-
- sscanf(buf, %llx, val);
-
- return (bool)val;
-}
-
static void min_max_config(void (*check)(void))
{
int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
@@ -493,8 +462,8 @@ static void reset(void)
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 04:38:24PM +, Chris Wilson wrote: Are you saying the Intel DDX currently derives a different meaning to the intented behaviour? in which case it can still be changed to not do that? I still disagree though. This provides all the information I need to support variable sized cursors and we can use large cursors today. Note that I won't fight if you find it useful and people are fine with that new meaning. Can we just throw a patch actually documented what you want those values to be? The WM patch still needs to take the actual cursor size though. Keeping those global limits on the crtc struct still looks weird. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] lib: add igt_get_stop_rings and igt_set_stop_rings
On Tue, Mar 25, 2014 at 07:02:21PM +0200, Mika Kuoppala wrote:
Multiple tests are introducing hangs by fidding with i915_ring_stop
debugfs entry.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Both patches look good to me. One thing we could do is add an exit handler
to reset stop_rings to 0, just in case the test died and somehow the
stop_rings value leaked.
With or w/o that change both patches lgtm, so feel free to push.
-Daniel
---
lib/igt_debugfs.c | 107
+
lib/igt_debugfs.h | 28 ++
2 files changed, 135 insertions(+)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index e04f8c5..c4cf50d 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -28,9 +28,11 @@
#include errno.h
#include stdio.h
#include stdlib.h
+#include limits.h
#include string.h
#include fcntl.h
#include unistd.h
+#include i915_drm.h
#include drmtest.h
#include igt_display.h
@@ -615,3 +617,108 @@ int igt_open_forcewake_handle(void)
{
return igt_debugfs_open(i915_forcewake_user, O_WRONLY);
}
+
+/**
+ * igt_to_stop_ring_flag:
+ * @ring: ring to get flag for, in drm I915_EXEC_* namespace
+ *
+ * This converts ring specified (drm namespace) to a ring flag
+ * to be used with igt_get_stop_rings() and igt_set_stop_rings().
+ *
+ * Returns:
+ * Ring flag for the give ring
+ */
+enum stop_ring_flags igt_to_stop_ring_flag(int ring) {
+ if (ring == I915_EXEC_DEFAULT)
+ return STOP_RING_RENDER;
+
+ igt_assert(ring ((ring ~I915_EXEC_RING_MASK) == 0));
+ return 1 (ring - 1);
+}
+
+static void stop_rings_write(uint32_t mask)
+{
+ int fd;
+ char buf[80];
+
+ igt_assert(snprintf(buf, sizeof(buf), 0x%08x, mask) == 10);
+ fd = igt_debugfs_open(i915_ring_stop, O_WRONLY);
+ igt_assert(fd = 0);
+
+ igt_assert(write(fd, buf, strlen(buf)) == strlen(buf));
+ close(fd);
+}
+
+/**
+ * igt_get_stop_rings:
+ *
+ * Read current ring flags from 'i915_ring_stop' debugfs entry.
+ *
+ * Returns:
+ * Current ring flags
+ */
+enum stop_ring_flags igt_get_stop_rings(void)
+{
+ int fd;
+ char buf[80];
+ int l;
+ unsigned long long ring_mask;
+
+ fd = igt_debugfs_open(i915_ring_stop, O_RDONLY);
+ igt_assert(fd = 0);
+ l = read(fd, buf, sizeof(buf));
+ igt_assert(l 0);
+ igt_assert(l sizeof(buf));
+
+ buf[l] = '\0';
+
+ close(fd);
+
+ errno = 0;
+ ring_mask = strtoull(buf, NULL, 0);
+ igt_assert(errno == 0);
+ return ring_mask;
+}
+
+/**
+ * igt_set_stop_rings:
+ * @flags: Ring flags to write
+ *
+ * This writes @flags to 'i915_ring_stop' debugfs entry. Driver will
+ * prevent cpu from writing tail pointer for ring @flags specify. Note
+ * that the ring is not stopped right away. Instead any further command
+ * emissions won't be executed after the flag is set.
+ *
+ * This is the least invasive way to make the gpu stuck. Hence you must
+ * set this after a batch submission with it's own invalid or endless
+ * looping instructions. In this case it is merely to give a log
+ * notification that this was simulated hang, as the batch would have
+ * caused hang in any case. Or if you use valid or noop batch and want
+ * to hang the ring (gpu), you must set this flag before submitting the
+ * batch.
+ *
+ * Driver checks periodically if ring is making any progress, and if
+ * not, it will declare the ring to be hung and will reset the GPU.
+ * After reset, the driver will clear flags in 'i915_ring_stop'
+ *
+ * Note: Always when hanging the gpu, use igt_set_stop_rings() to
+ * notify the driver. Driver tunes down gpu hang log messaging
+ * based on these flags and thus prevents false positives on logs.
+ *
+ */
+void igt_set_stop_rings(enum stop_ring_flags flags)
+{
+ enum stop_ring_flags current;
+
+ igt_assert((flags ~STOP_RING_ALL) == 0);
+
+ current = igt_get_stop_rings();
+ igt_assert_f(current == 0,
+ previous i915_ring_stop is still 0x%x\n, current);
+
+ stop_rings_write(flags);
+ current = igt_get_stop_rings();
+ if (current != flags)
+ igt_warn(i915_ring_stop readback mismatch 0x%x vs 0x%x\n,
+ flags, current);
+}
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 3312a8b..72f4999 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -143,4 +143,32 @@ void igt_enable_prefault(void);
int igt_open_forcewake_handle(void);
+/**
+ * stop_ring_flags:
+ *
+ * @STOP_RING_NONE: Can be used to clear the pending stop (warning: hang
might
+ * be declared already). Returned by igt_get_stop_rings() if there is
+ * no currently stopped rings.
+ * @STOP_RING_RENDER: Render ring
+ * @STOP_RING_BSD: Video encoding/decoding ring
+ * @STOP_RING_BLT: Blitter ring
+ * @STOP_RING_VEBOX: Video enchanment ring
+ * @STOP_RING_ALL:
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 04:57:04PM +, Damien Lespiau wrote:
On Tue, Mar 25, 2014 at 04:38:24PM +, Chris Wilson wrote:
For the record,
16:30 agd5f ickle, our GPUs don't have selectable cursor sizes
16:31 agd5f so on the newer ones, xf86-video-modesetting, etc. would
allocate a 64x64 cursor and it would look squashed and funky since the
hw expects 128x128
Which means I was confused when I thought part of the reasoning was
indeed HiDPI support. (I'm still seem to remember that was part of the
argument for large cursors anyway.)
Are you saying the Intel DDX currently derives a different meaning to
the intented behaviour? in which case it can still be changed to not do
that?
I still disagree though. This provides all the information I need to
support variable sized cursors and we can use large cursors today.
I'd love the game to be about defining clear semantics more than by
interpreting that value this way, I got what I always wanted :)
We can resolve that today with MAX_CURSOR_WIDTH, MAX_CURSOR_HEIGHT caps
as well (if you're alluding at the fact that drm_planes may still be a
few decades away).
We'll still need to expose the full list of supported cursor sizes for
compositors at some point or another, my preferred way would be with a
property in the exposed cursor drm_plane.
For the record I'll restate my comment here about the larger problem:
Atm we have no way for userspace to figure out per-plane limits on
sizes/strides, we only expose a lists of supported pixel formats.
Imo exposing cursor limits is just part of this larger issue, so if we can
get the current stuff working with some legalese, then I'm ok with that.
Solving the larger issue is much more work, and it's better to have a few
more odd cases working than not. For planes I guess we could have a few
limits:
min/max width height in pixels
min/max stride
alignment of stride
And a pile of flags
- needs pot stride/width/height
- needs square size
That should be enough to cover most single-plane things. For multiplanar I
guess we might either just need 2nd/3rd set of those or some additional
stuff expressed in flags (e.g. subsampled strides must be half of the Y
stride). Or we'll throw our hands in the air for multi-planar for now ;-)
Or we simply do this per-pixel format with one for each framebuffer plane,
i.e.
struct drm_get_plane_fb_limits {
uint32_t plane_id; /* in */
uint32_t fourcc; /* in */
struct drm_plane_limits limits[MAX_FOURCC_PLANES];
/* the stuff above for all possible planes of a fourcc code */
}
Saner drivers could then return the same thing for all fourccs codes in
their backend.
Just something to ponder.
Adding dri-devel, maybe we can get this discussion started.
-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/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 07:23:26PM +0100, Daniel Vetter wrote:
On Tue, Mar 25, 2014 at 04:57:04PM +, Damien Lespiau wrote:
On Tue, Mar 25, 2014 at 04:38:24PM +, Chris Wilson wrote:
For the record,
16:30 agd5f ickle, our GPUs don't have selectable cursor sizes
16:31 agd5f so on the newer ones, xf86-video-modesetting, etc. would
allocate a 64x64 cursor and it would look squashed and funky since the
hw expects 128x128
Which means I was confused when I thought part of the reasoning was
indeed HiDPI support. (I'm still seem to remember that was part of the
argument for large cursors anyway.)
Are you saying the Intel DDX currently derives a different meaning to
the intented behaviour? in which case it can still be changed to not do
that?
I still disagree though. This provides all the information I need to
support variable sized cursors and we can use large cursors today.
I'd love the game to be about defining clear semantics more than by
interpreting that value this way, I got what I always wanted :)
We can resolve that today with MAX_CURSOR_WIDTH, MAX_CURSOR_HEIGHT caps
as well (if you're alluding at the fact that drm_planes may still be a
few decades away).
We'll still need to expose the full list of supported cursor sizes for
compositors at some point or another, my preferred way would be with a
property in the exposed cursor drm_plane.
For the record I'll restate my comment here about the larger problem:
Atm we have no way for userspace to figure out per-plane limits on
sizes/strides, we only expose a lists of supported pixel formats.
Imo exposing cursor limits is just part of this larger issue, so if we can
get the current stuff working with some legalese, then I'm ok with that.
Solving the larger issue is much more work, and it's better to have a few
more odd cases working than not. For planes I guess we could have a few
limits:
min/max width height in pixels
min/max stride
alignment of stride
And a pile of flags
- needs pot stride/width/height
- needs square size
Already new ones:
- stride must match width*bpp
This will be fun if we ever do it ...
-Daniel
That should be enough to cover most single-plane things. For multiplanar I
guess we might either just need 2nd/3rd set of those or some additional
stuff expressed in flags (e.g. subsampled strides must be half of the Y
stride). Or we'll throw our hands in the air for multi-planar for now ;-)
Or we simply do this per-pixel format with one for each framebuffer plane,
i.e.
struct drm_get_plane_fb_limits {
uint32_t plane_id; /* in */
uint32_t fourcc; /* in */
struct drm_plane_limits limits[MAX_FOURCC_PLANES];
/* the stuff above for all possible planes of a fourcc code */
}
Saner drivers could then return the same thing for all fourccs codes in
their backend.
Just something to ponder.
Adding dri-devel, maybe we can get this discussion started.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] quick_dump: Fix the danvet fallout.
quick_dump built fine, but it could actually run, since a lot of the
linking happens at run time. There is one hack where we redefine the
environment stuff, since depending on igt_aux means we have to pull in
libdrm, which I do not want to do.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/quick_dump/Makefile.am | 2 ++
tools/quick_dump/chipset.i| 5 +++--
tools/quick_dump/chipset_macro_wrap.c | 14 ++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/tools/quick_dump/Makefile.am b/tools/quick_dump/Makefile.am
index 7572ee5..ca26993 100644
--- a/tools/quick_dump/Makefile.am
+++ b/tools/quick_dump/Makefile.am
@@ -8,6 +8,8 @@ bin_SCRIPTS = chipset.py
lib_LTLIBRARIES = I915ChipsetPython.la
I915ChipsetPython_la_LDFLAGS = -module -avoid-version $(PYTHON_LDFLAGS)
$(PCIACCESS_LIBS)
I915ChipsetPython_la_SOURCES = chipset_wrap_python.c chipset_macro_wrap.c \
+ $(top_srcdir)/lib/igt_core.c \
+ $(top_srcdir)/lib/igt_debugfs.c \
$(top_srcdir)/lib/intel_os.c \
$(top_srcdir)/lib/intel_chipset.c \
$(top_srcdir)/lib/intel_reg_map.c \
diff --git a/tools/quick_dump/chipset.i b/tools/quick_dump/chipset.i
index 395418e..ae176e8 100644
--- a/tools/quick_dump/chipset.i
+++ b/tools/quick_dump/chipset.i
@@ -4,6 +4,7 @@
#include pciaccess.h
#include stdint.h
#include intel_chipset.h
+#include intel_io.h
extern int is_sandybridge(unsigned short pciid);
extern int is_ivybridge(unsigned short pciid);
extern int is_valleyview(unsigned short pciid);
@@ -12,7 +13,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
@@ -27,7 +28,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
diff --git a/tools/quick_dump/chipset_macro_wrap.c
b/tools/quick_dump/chipset_macro_wrap.c
index 392b85e..ee79777 100644
--- a/tools/quick_dump/chipset_macro_wrap.c
+++ b/tools/quick_dump/chipset_macro_wrap.c
@@ -1,3 +1,5 @@
+#include stdbool.h
+#include stdlib.h
#include pciaccess.h
#include intel_chipset.h
@@ -31,3 +33,15 @@ unsigned short pcidev_to_devid(struct pci_device *pdev)
{
return pdev-device_id;
}
+
+bool igt_check_boolean_env_var(const char *env_var, bool default_value)
+{
+ char *val;
+
+ val = getenv(env_var);
+ if (!val)
+ return default_value;
+
+ return atoi(val) != 0;
+}
+
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 07:23:26PM +0100, Daniel Vetter wrote:
Or we simply do this per-pixel format with one for each framebuffer plane,
i.e.
struct drm_get_plane_fb_limits {
uint32_t plane_id; /* in */
uint32_t fourcc; /* in */
struct drm_plane_limits limits[MAX_FOURCC_PLANES];
/* the stuff above for all possible planes of a fourcc code */
}
Saner drivers could then return the same thing for all fourccs codes in
their backend.
Some of the limits are definitely per format. Plane max dimensions are a
good example of a limit that can change per-format (8bpp Vs 10bpp to be
contained within the same max bandwidth of the hw).
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] Convert last few uses of __FUNCTION__ to __func__
Outside of staging, there aren't any more uses of __FUNCTION__ now... Joe Perches (5): powerpc: Convert last uses of __FUNCTION__ to __func__ x86: Convert last uses of __FUNCTION__ to __func__ block: Convert last uses of __FUNCTION__ to __func__ i915: Convert last uses of __FUNCTION__ to __func__ slab: Convert last uses of __FUNCTION__ to __func__ arch/powerpc/platforms/pseries/nvram.c | 11 +-- arch/x86/kernel/hpet.c | 2 +- arch/x86/kernel/rtc.c| 4 ++-- arch/x86/platform/intel-mid/intel_mid_vrtc.c | 2 +- drivers/block/drbd/drbd_int.h| 8 drivers/block/xen-blkfront.c | 4 ++-- drivers/gpu/drm/i915/dvo_ns2501.c| 15 ++- mm/slab.h| 2 +- 8 files changed, 22 insertions(+), 26 deletions(-) -- 1.8.1.2.459.gbcd45b4.dirty ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Don't set mode_config's cursor size
On Tue, Mar 25, 2014 at 7:51 PM, Damien Lespiau
damien.lesp...@intel.com wrote:
On Tue, Mar 25, 2014 at 07:23:26PM +0100, Daniel Vetter wrote:
Or we simply do this per-pixel format with one for each framebuffer plane,
i.e.
struct drm_get_plane_fb_limits {
uint32_t plane_id; /* in */
uint32_t fourcc; /* in */
struct drm_plane_limits limits[MAX_FOURCC_PLANES];
/* the stuff above for all possible planes of a fourcc code */
}
Saner drivers could then return the same thing for all fourccs codes in
their backend.
Some of the limits are definitely per format. Plane max dimensions are a
good example of a limit that can change per-format (8bpp Vs 10bpp to be
contained within the same max bandwidth of the hw).
One thing I've completely missed btw is scaling limits. How we then
need to factor in bandwidth I have no idea about ... I guess at one
point it boils down to try it and give up if it doesn't work. And I
think we need a few scaling related flags like can't scale at all or
sub-sampling restrictions. Who knows ...
-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 4/5] i915: Convert last uses of __FUNCTION__ to __func__
Just about all of these have been converted to __func__,
so convert the last uses.
Signed-off-by: Joe Perches j...@perches.com
---
drivers/gpu/drm/i915/dvo_ns2501.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 954acb2..e40cd26 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -234,7 +234,7 @@ static enum drm_mode_status ns2501_mode_valid(struct
intel_dvo_device *dvo,
{
DRM_DEBUG_KMS
(%s: is mode valid
(hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d)\n,
-__FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+__func__, mode-hdisplay, mode-htotal, mode-vdisplay,
mode-vtotal);
/*
@@ -262,7 +262,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
DRM_DEBUG_KMS
(%s: set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n,
-__FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+__func__, mode-hdisplay, mode-htotal, mode-vdisplay,
mode-vtotal);
/*
@@ -277,8 +277,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
if (mode-hdisplay == 800 mode-vdisplay == 600) {
/* mode 277 */
ns-reg_8_shadow = ~NS2501_8_BPAS;
- DRM_DEBUG_KMS(%s: switching to 800x600\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 800x600\n, __func__);
/*
* No, I do not know where this data comes from.
@@ -341,8 +340,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
} else if (mode-hdisplay == 640 mode-vdisplay == 480) {
/* mode 274 */
- DRM_DEBUG_KMS(%s: switching to 640x480\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 640x480\n, __func__);
/*
* No, I do not know where this data comes from.
* It is just what the video bios left in the DVO, so
@@ -406,8 +404,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
} else if (mode-hdisplay == 1024 mode-vdisplay == 768) {
/* mode 280 */
- DRM_DEBUG_KMS(%s: switching to 1024x768\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 1024x768\n, __func__);
/*
* This might or might not work, actually. I'm silently
* assuming here that the native panel resolution is
@@ -459,7 +456,7 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool
enable)
unsigned char ch;
DRM_DEBUG_KMS(%s: Trying set the dpms of the DVO to %i\n,
- __FUNCTION__, enable);
+ __func__, enable);
ch = ns-reg_8_shadow;
--
1.8.1.2.459.gbcd45b4.dirty
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/13] Gen7 batch buffer command parser
On Tue, Mar 25, 2014 at 06:15:36AM -0700, Daniel Vetter wrote: On Thu, Mar 20, 2014 at 04:43:05PM +0200, Jani Nikula wrote: Hi Bradley - Apologies for my procrastination with the review; I don't easily recall as tedious a review as the command and register tables. And I sure have reviewed a lot of miserable stuff in the past. Most infuriatingly, I did not find a single real bug in the code! I think we'll need to automate some things going forward, for example listing the non-conforming length encoding with Damien's tools for cross checking. There are a few subtle points we need to discuss (separate mails internally) but all in all this series is: Reviewed-by: Jani Nikula jani.nik...@intel.com Ok, pulled this one in, thanks a lot for the patchesreview. I think it's time we start to move on with the next bits, the batch copy stuff seams like a suitable piece. There's still issues with launching the entire thing in the end, but we can start with the copy infrastructure. Open issues I see still: - The littel issue we're discussing internally. Like I've said that one is blocking us and needs to be resolved before we can switch to enforcing mode. - Secure batch dispatch is still fubar. I'm not sure that this will still impact us once we implement the batch copy step. I was only using the secure dispatch stuff because it was a convenient way to get the batch into GGTT. But with the copy step, we could just have separate code to do that. - CodingStyle says: Functions should be a) at most 3 indent levels b) at most 3 ansi screens long (i.e. 75 lines). i915_parse_cmds violates both metrics pretty deftly. I think a few refactoring patches to extract helper functions and structure the flow a bit would be good. :) I'll start with a patch to move all of the actual checking logic into a separate function, with maybe an extra helper for the bitmask checks. That seems like it should cut the size down sufficiently. Brad 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 4/5] i915: Convert last uses of __FUNCTION__ to __func__
On Tue, Mar 25, 2014 at 12:35:06PM -0700, Joe Perches wrote:
Just about all of these have been converted to __func__,
so convert the last uses.
Signed-off-by: Joe Perches j...@perches.com
Pulled into drm-intel, should land in 3.15.
Thanks, Daniel
---
drivers/gpu/drm/i915/dvo_ns2501.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c
b/drivers/gpu/drm/i915/dvo_ns2501.c
index 954acb2..e40cd26 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -234,7 +234,7 @@ static enum drm_mode_status ns2501_mode_valid(struct
intel_dvo_device *dvo,
{
DRM_DEBUG_KMS
(%s: is mode valid
(hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d)\n,
- __FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+ __func__, mode-hdisplay, mode-htotal, mode-vdisplay,
mode-vtotal);
/*
@@ -262,7 +262,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
DRM_DEBUG_KMS
(%s: set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n,
- __FUNCTION__, mode-hdisplay, mode-htotal, mode-vdisplay,
+ __func__, mode-hdisplay, mode-htotal, mode-vdisplay,
mode-vtotal);
/*
@@ -277,8 +277,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
if (mode-hdisplay == 800 mode-vdisplay == 600) {
/* mode 277 */
ns-reg_8_shadow = ~NS2501_8_BPAS;
- DRM_DEBUG_KMS(%s: switching to 800x600\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 800x600\n, __func__);
/*
* No, I do not know where this data comes from.
@@ -341,8 +340,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
} else if (mode-hdisplay == 640 mode-vdisplay == 480) {
/* mode 274 */
- DRM_DEBUG_KMS(%s: switching to 640x480\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 640x480\n, __func__);
/*
* No, I do not know where this data comes from.
* It is just what the video bios left in the DVO, so
@@ -406,8 +404,7 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
} else if (mode-hdisplay == 1024 mode-vdisplay == 768) {
/* mode 280 */
- DRM_DEBUG_KMS(%s: switching to 1024x768\n,
- __FUNCTION__);
+ DRM_DEBUG_KMS(%s: switching to 1024x768\n, __func__);
/*
* This might or might not work, actually. I'm silently
* assuming here that the native panel resolution is
@@ -459,7 +456,7 @@ static void ns2501_dpms(struct intel_dvo_device *dvo,
bool enable)
unsigned char ch;
DRM_DEBUG_KMS(%s: Trying set the dpms of the DVO to %i\n,
- __FUNCTION__, enable);
+ __func__, enable);
ch = ns-reg_8_shadow;
--
1.8.1.2.459.gbcd45b4.dirty
--
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] quick_dump: Fix the danvet fallout.
On Tue, Mar 25, 2014 at 11:38:51AM -0700, Ben Widawsky wrote:
quick_dump built fine, but it could actually run, since a lot of the
linking happens at run time. There is one hack where we redefine the
environment stuff, since depending on igt_aux means we have to pull in
libdrm, which I do not want to do.
Why? libdrm is kinda something we need anyway ...
-Daniel
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/quick_dump/Makefile.am | 2 ++
tools/quick_dump/chipset.i| 5 +++--
tools/quick_dump/chipset_macro_wrap.c | 14 ++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/tools/quick_dump/Makefile.am b/tools/quick_dump/Makefile.am
index 7572ee5..ca26993 100644
--- a/tools/quick_dump/Makefile.am
+++ b/tools/quick_dump/Makefile.am
@@ -8,6 +8,8 @@ bin_SCRIPTS = chipset.py
lib_LTLIBRARIES = I915ChipsetPython.la
I915ChipsetPython_la_LDFLAGS = -module -avoid-version $(PYTHON_LDFLAGS)
$(PCIACCESS_LIBS)
I915ChipsetPython_la_SOURCES = chipset_wrap_python.c chipset_macro_wrap.c \
+$(top_srcdir)/lib/igt_core.c \
+$(top_srcdir)/lib/igt_debugfs.c \
$(top_srcdir)/lib/intel_os.c \
$(top_srcdir)/lib/intel_chipset.c \
$(top_srcdir)/lib/intel_reg_map.c \
diff --git a/tools/quick_dump/chipset.i b/tools/quick_dump/chipset.i
index 395418e..ae176e8 100644
--- a/tools/quick_dump/chipset.i
+++ b/tools/quick_dump/chipset.i
@@ -4,6 +4,7 @@
#include pciaccess.h
#include stdint.h
#include intel_chipset.h
+#include intel_io.h
extern int is_sandybridge(unsigned short pciid);
extern int is_ivybridge(unsigned short pciid);
extern int is_valleyview(unsigned short pciid);
@@ -12,7 +13,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
@@ -27,7 +28,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
diff --git a/tools/quick_dump/chipset_macro_wrap.c
b/tools/quick_dump/chipset_macro_wrap.c
index 392b85e..ee79777 100644
--- a/tools/quick_dump/chipset_macro_wrap.c
+++ b/tools/quick_dump/chipset_macro_wrap.c
@@ -1,3 +1,5 @@
+#include stdbool.h
+#include stdlib.h
#include pciaccess.h
#include intel_chipset.h
@@ -31,3 +33,15 @@ unsigned short pcidev_to_devid(struct pci_device *pdev)
{
return pdev-device_id;
}
+
+bool igt_check_boolean_env_var(const char *env_var, bool default_value)
+{
+ char *val;
+
+ val = getenv(env_var);
+ if (!val)
+ return default_value;
+
+ return atoi(val) != 0;
+}
+
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] tests/gem_exec_parse: Test for batches w/o MI_BATCH_BUFFER_END
On Tue, Mar 25, 2014 at 06:17:55AM -0700, Daniel Vetter wrote:
On Thu, Jan 30, 2014 at 11:46:15AM +, Chris Wilson wrote:
On Wed, Jan 29, 2014 at 10:10:47PM +, Chris Wilson wrote:
On Wed, Jan 29, 2014 at 01:58:29PM -0800, bradley.d.vol...@intel.com
wrote:
From: Brad Volkin bradley.d.vol...@intel.com
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
tests/gem_exec_parse.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 9e90408..004c3bf 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -257,6 +257,15 @@ igt_main
-EINVAL));
}
+ igt_subtest(batch-without-end) {
+ uint32_t noop[1024] = { 0 };
+ igt_assert(
+ exec_batch(fd, handle,
+ noop, sizeof(noop),
+ I915_EXEC_RENDER,
+ -EINVAL));
Cheekier would be
uint32_t empty[] = { MI_NOOP, MI_NOOP, MI_BATCH_BUFFER_END, 0 };
for_each_ring() {
igt_assert(exec_batch(fd, handle, empty, sizeof(empty), ring, 0));
igt_assert(exec_batch(fd, handle, empty, 8, ring, -EINVAL));
}
On this subject, it should be
{ INVALID, INVALID, NOOP, NOOP, END, 0}
assert(exec(0, 4) == -EINVAL);
assert(exec(0, 8) == -EINVAL);
assert(exec(0, 12) == -EINVAL);
assert(exec(4, 8) == -EINVAL);
assert(exec(4, 12) == 0);
assert(exec(8, 12) == 0);
Brad, care to throw this nasties into the test pond, too?
Yeah, I can add that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/13] Gen7 batch buffer command parser
On Tue, Mar 25, 2014 at 12:46:37PM -0700, Volkin, Bradley D wrote: On Tue, Mar 25, 2014 at 06:15:36AM -0700, Daniel Vetter wrote: - Secure batch dispatch is still fubar. I'm not sure that this will still impact us once we implement the batch copy step. I was only using the secure dispatch stuff because it was a convenient way to get the batch into GGTT. But with the copy step, we could just have separate code to do that. The problem isn't copying or allocating the bo, the issue is running it with a) the hw checker disabled b) not mapped into any ppgtt so hidden from all (unchecked) access and c) otherwise working like a normal batch. For that we need to employ the secure batch dispatch code in the execbuf code. Atm b) is broken for aliasing ppgtt and c) is broken for full ppgtt. So a bit of blockers for us. But at least broken b) with aliasing ppgtt is kinda a regression, which means I'll get around to it soonish (before 3.15-rc1 at least). -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: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 05:52:00PM +0100, Daniel Vetter wrote: On Tue, Mar 25, 2014 at 5:29 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: Yeah I've seen the other patches. I think we should try to keep all the ring structures around even when the hw init failed. I've made some feeble attempts a while ago to split the structure init from the hw init stuff, but kinda never fully materialized ... Imo if our set of valid rings semi-randomly changes at runtime even, that's not good. Agreed, but sadly we can't trust hardware to always work, and we need something to prevent explosions. I quite like the idea of marking the GPU wedged if hw init fails so that we lose acceleration but keep modesetting around. Yeah, I agree that the other two patches are neat indeed, it's this one here where the shiny starts to come off a bit ;-) tbh I'd prefer a simply if (terminally_wedged) return -EIO; here before the ring checks, maybe with a comment stating why we need to have this order. It's ok, it is only to prevent UXA from going off the rails after the odd resume hang on g45... That, or fix the mess called ring init code ... So if we fixed resume to avoid reallocating the ringbuffers across resume, g45 would still fail to restart, but now we still have valid objects (or would we tear them down because of the failure?) and so this check passes and we later hit the EIO checks? -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] quick_dump: Fix the danvet fallout.
On Tue, Mar 25, 2014 at 08:47:47PM +0100, Daniel Vetter wrote:
On Tue, Mar 25, 2014 at 11:38:51AM -0700, Ben Widawsky wrote:
quick_dump built fine, but it could actually run, since a lot of the
linking happens at run time. There is one hack where we redefine the
environment stuff, since depending on igt_aux means we have to pull in
libdrm, which I do not want to do.
Why? libdrm is kinda something we need anyway ...
-Daniel
The goal is to keep the dumper's dependencies and functionality to a
bare minimum. We absolutely should not need libdrm for register dumps.
If you want to do fancy stuff post process that requires libdrm, that's
another thing.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
tools/quick_dump/Makefile.am | 2 ++
tools/quick_dump/chipset.i| 5 +++--
tools/quick_dump/chipset_macro_wrap.c | 14 ++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/tools/quick_dump/Makefile.am b/tools/quick_dump/Makefile.am
index 7572ee5..ca26993 100644
--- a/tools/quick_dump/Makefile.am
+++ b/tools/quick_dump/Makefile.am
@@ -8,6 +8,8 @@ bin_SCRIPTS = chipset.py
lib_LTLIBRARIES = I915ChipsetPython.la
I915ChipsetPython_la_LDFLAGS = -module -avoid-version $(PYTHON_LDFLAGS)
$(PCIACCESS_LIBS)
I915ChipsetPython_la_SOURCES = chipset_wrap_python.c chipset_macro_wrap.c \
+ $(top_srcdir)/lib/igt_core.c \
+ $(top_srcdir)/lib/igt_debugfs.c \
$(top_srcdir)/lib/intel_os.c \
$(top_srcdir)/lib/intel_chipset.c \
$(top_srcdir)/lib/intel_reg_map.c \
diff --git a/tools/quick_dump/chipset.i b/tools/quick_dump/chipset.i
index 395418e..ae176e8 100644
--- a/tools/quick_dump/chipset.i
+++ b/tools/quick_dump/chipset.i
@@ -4,6 +4,7 @@
#include pciaccess.h
#include stdint.h
#include intel_chipset.h
+#include intel_io.h
extern int is_sandybridge(unsigned short pciid);
extern int is_ivybridge(unsigned short pciid);
extern int is_valleyview(unsigned short pciid);
@@ -12,7 +13,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int
safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
@@ -27,7 +28,7 @@ extern int is_broadwell(unsigned short pciid);
extern struct pci_device *intel_get_pci_device();
extern int intel_register_access_init(struct pci_device *pci_dev, int
safe);
extern uint32_t intel_register_read(uint32_t reg);
-extern uint32_t intel_register_write(uint32_t reg, uint32_t val);
+extern void intel_register_write(uint32_t reg, uint32_t val);
extern void intel_register_access_fini();
extern int intel_register_access_needs_fakewake();
extern unsigned short pcidev_to_devid(struct pci_device *pci_dev);
diff --git a/tools/quick_dump/chipset_macro_wrap.c
b/tools/quick_dump/chipset_macro_wrap.c
index 392b85e..ee79777 100644
--- a/tools/quick_dump/chipset_macro_wrap.c
+++ b/tools/quick_dump/chipset_macro_wrap.c
@@ -1,3 +1,5 @@
+#include stdbool.h
+#include stdlib.h
#include pciaccess.h
#include intel_chipset.h
@@ -31,3 +33,15 @@ unsigned short pcidev_to_devid(struct pci_device *pdev)
{
return pdev-device_id;
}
+
+bool igt_check_boolean_env_var(const char *env_var, bool default_value)
+{
+ char *val;
+
+ val = getenv(env_var);
+ if (!val)
+ return default_value;
+
+ return atoi(val) != 0;
+}
+
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Tue, Mar 25, 2014 at 10:33 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: That, or fix the mess called ring init code ... So if we fixed resume to avoid reallocating the ringbuffers across resume, g45 would still fail to restart, but now we still have valid objects (or would we tear them down because of the failure?) and so this check passes and we later hit the EIO checks? Yeah that's my idea: We always set up the objects for all valid rings and never tear them down. Upon failure we just set wedged and execbuf would fall through until it notices the wedged states and returns -EIO. Gives us tidy unified code and no special-casing for the ring init failures. And if we do the split in ring init correctly the ring_hw_init could unconditionally set wedged internally if it fails and our code would dtrt no matter whether it's driver load, resume or failure to resurrect the hw after reset. So callers wouldn't need to care. Of course the ring_init functions which allocates the structures in driver load still needs to be able to bail out in case that fails. I can dream ... -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] drm/i915: Prevent context obj from being corrupted
While the context is not being used, we can make the PTEs invalid, so
nothing can accidentally corrupt it. Systems tend to have a lot of
trouble when the context gets corrupted.
NOTE: This is a slightly different patch than what I posted to Bugzilla.
References: https://bugs.freedesktop.org/show_bug.cgi?id=75724
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_context.c | 56 +
drivers/gpu/drm/i915/i915_reg.h | 2 +-
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 6043062..4405a92 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -584,6 +584,58 @@ i915_gem_context_get(struct drm_i915_file_private
*file_priv, u32 id)
return ctx;
}
+static void
+_ctx_ptes(struct intel_ring_buffer *ring,
+ struct i915_hw_context *ctx,
+ bool valid)
+{
+ const size_t ptes = ctx-obj-base.size PAGE_SHIFT;
+ const u32 base = i915_gem_obj_ggtt_offset(ctx-obj);
+ struct sg_page_iter sg_iter;
+ struct i915_address_space *vm = ctx-vm;
+ int i = 0;
+
+ BUG_ON(!i915_gem_obj_is_pinned(ctx-obj));
+
+ if (intel_ring_begin(ring, round_up(ptes * 3, 2))) {
+ DRM_ERROR(Could not protect context object.\n);
+ return;
+ }
+
+ for_each_sg_page(ctx-obj-pages-sgl, sg_iter,
ctx-obj-pages-nents, 0) {
+ u32 pte = vm-pte_encode(sg_page_iter_dma_address(sg_iter),
+ctx-obj-cache_level,
+valid);
+ intel_ring_emit(ring, MI_UPDATE_GTT | (122));
+ /* The docs contradict themselves on the offset. They say dword
+* offset, yet the low 12 bits MBZ. */
+ intel_ring_emit(ring, (base PAGE_MASK) + i);
+ intel_ring_emit(ring, pte);
+ i+=PAGE_SIZE;
+ }
+
+ if (i PAGE_SHIFT)
+ intel_ring_emit(ring, MI_NOOP);
+
+ intel_ring_advance(ring);
+}
+
+static void
+enable_ctx_ptes(struct intel_ring_buffer *ring,
+ struct i915_hw_context *ctx)
+{
+ if (INTEL_INFO(ring-dev)-gen 8)
+ _ctx_ptes(ring, ctx, true);
+}
+
+static void
+disable_ctx_ptes(struct intel_ring_buffer *ring,
+struct i915_hw_context *ctx)
+{
+ if (INTEL_INFO(ring-dev)-gen 8)
+ _ctx_ptes(ring, ctx, false);
+}
+
static inline int
mi_set_context(struct intel_ring_buffer *ring,
struct i915_hw_context *new_context,
@@ -602,6 +654,8 @@ mi_set_context(struct intel_ring_buffer *ring,
return ret;
}
+ enable_ctx_ptes(ring, new_context);
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -632,6 +686,8 @@ mi_set_context(struct intel_ring_buffer *ring,
intel_ring_advance(ring);
+ disable_ctx_ptes(ring, new_context);
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9f9e2b7..d536706 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -367,7 +367,7 @@
#define MI_TOPOLOGY_FILTER MI_INSTR(0x0D, 0)
#define MI_LOAD_SCAN_LINES_EXCL MI_INSTR(0x13, 0)
#define MI_URB_CLEARMI_INSTR(0x19, 0)
-#define MI_UPDATE_GTT MI_INSTR(0x23, 0)
+#define MI_UPDATE_GTT MI_INSTR(0x23, 1)
#define MI_CLFLUSH MI_INSTR(0x27, 0)
#define MI_REPORT_PERF_COUNTMI_INSTR(0x28, 0)
#define MI_REPORT_PERF_COUNT_GGTT (10)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Enable FBC on GEN7 by default
On Tue, Mar 25, 2014 at 12:27 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Mar 24, 2014 at 06:21:22PM -0700, Ben Widawsky wrote: I am not clear why we've never enabled it by default for GEN7. Looking at the git hostiry, it seems Rodrigo disabled it by default, and it's never been turned on. Quite a few fixes have gone in over the past year, and I think many of us are running this successfully. If there is some reason we know of why we don't enable this by default on GEN7, then please ignore the patch, and forgive my laziness. Other than the major performance degredation due to our implementation, That sounds interesting, can you elaborate on the performance degradation? I haven't noticed any, but of course I don't know how it's supposed to behave... Stéphane ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add property to set HDMI aspect ratio
Added a property to enable user space to set aspect ratio for HDMI displays.
If there is no user specified value, then PAR_NONE/Automatic option is set
by default. User can select aspect ratio 4:3 or 16:9. The aspect ratio
selected by user would come into effect with a mode set.
v2: Daniel's review comments incorporated.
Call for a mode set to update property.
Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Jesse Barnes jesse.bar...@intel.com
Cc: Vijay Purushothaman vijay.a.purushotha...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h|1 +
drivers/gpu/drm/i915/intel_drv.h |2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 12
drivers/gpu/drm/i915/intel_modes.c | 28
4 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b8c1e0..628ba2e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1588,6 +1588,7 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ struct drm_property *aspect_ratio_property;
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fa99104..262142f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -474,6 +474,7 @@ struct intel_hdmi {
bool has_audio;
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
+ enum hdmi_picture_aspect aspect_ratio;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
@@ -834,6 +835,7 @@ int intel_connector_update_modes(struct drm_connector
*connector,
int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_aspect_ratio_property(struct drm_connector *connector);
/* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index b0413e1..0b99485 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder
*encoder,
union hdmi_infoframe frame;
int ret;
+ /* Set user selected PAR to incoming mode's member */
+ adjusted_mode-picture_aspect_ratio = intel_hdmi-aspect_ratio;
+
ret = drm_hdmi_avi_infoframe_from_display_mode(frame.avi,
adjusted_mode);
if (ret 0) {
@@ -1094,6 +1097,11 @@ intel_hdmi_set_property(struct drm_connector *connector,
goto done;
}
+ if (property == dev_priv-aspect_ratio_property) {
+ intel_hdmi-aspect_ratio = val;
+ goto done;
+ }
+
return -EINVAL;
done:
@@ -1227,6 +1235,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
{
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
+ intel_attach_aspect_ratio_property(connector);
intel_hdmi-color_range_auto = true;
}
@@ -1291,6 +1300,9 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
intel_connector-get_hw_state = intel_connector_get_hw_state;
intel_connector-unregister = intel_connector_unregister;
+ /* Initialize aspect ratio member of intel_hdmi */
+ intel_hdmi-aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
intel_hdmi_add_properties(intel_hdmi, connector);
intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_modes.c
b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..6f814da 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,31 @@ intel_attach_broadcast_rgb_property(struct drm_connector
*connector)
drm_object_attach_property(connector-base, prop, 0);
}
+
+static const struct drm_prop_enum_list aspect_ratio_names[] = {
+ { HDMI_PICTURE_ASPECT_NONE, Automatic },
+ { HDMI_PICTURE_ASPECT_4_3, 4:3 },
+ { HDMI_PICTURE_ASPECT_16_9, 16:9 },
+};
+
+void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_property *prop;
+
+ prop = dev_priv-aspect_ratio_property;
+ if (prop == NULL) {
+
[Intel-gfx] [PATCH v3 2/3] drm/i915: New drm crtc property for varying the Pipe Src size
From: Akash Goel akash.g...@intel.com
This patch adds a new drm crtc property for varying the Pipe Src size
or the Panel fitter input size. Pipe Src controls the size that is
scaled from.
This will allow to dynamically flip (without modeset) the frame buffers
of different resolutions
v2: Added a check to fail the set property call if Panel fitter is
disabled new PIPESRC programming do not match with PIPE timings.
Removed the pitch mismatch check on frame buffer, when being flipped.
This is currently done only for VLV/HSW.
v3: Modified the check, added in v2, to consider the platforms having
Split PCH.
Testcase: igt/kms_panel_fitter_test
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 6
drivers/gpu/drm/i915/intel_display.c | 65 ++--
2 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0d90ef..6f3af15 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1608,6 +1608,12 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ /*
+* Property to dynamically vary the size of the
+* PIPESRC or Panel fitter input size
+*/
+ struct drm_property *input_size_property;
+
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5dfe156..7149123 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8935,8 +8935,18 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
* Note that pitch changes could also affect these register.
*/
if (INTEL_INFO(dev)-gen 3
- (fb-offsets[0] != crtc-fb-offsets[0] ||
-fb-pitches[0] != crtc-fb-pitches[0]))
+ (fb-offsets[0] != crtc-fb-offsets[0]))
+ return -EINVAL;
+
+ /*
+* Bypassing the fb pitch check for VLV/HSW, as purportedly there
+* is a dynamic flip support in VLV/HSW. This will allow to
+* flip fbs of different resolutions without doing a modeset.
+* TBD, confirm the same for other newer gen platforms also.
+*/
+ if (INTEL_INFO(dev)-gen 3
+ !IS_VALLEYVIEW(dev) !IS_HASWELL(dev)
+ (fb-pitches[0] != crtc-fb-pitches[0]))
return -EINVAL;
if (i915_terminally_wedged(dev_priv-gpu_error))
@@ -10434,8 +10444,50 @@ out_config:
static int intel_crtc_set_property(struct drm_crtc *crtc,
struct drm_property *property, uint64_t val)
{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret = -ENOENT;
+ if (property == dev_priv-input_size_property) {
+ int new_width = (int)((val 16) 0x);
+ int new_height = (int)(val 0x);
+ const struct drm_display_mode *adjusted_mode =
+ intel_crtc-config.adjusted_mode;
+
+ if ((new_width == intel_crtc-config.pipe_src_w)
+ (new_height == intel_crtc-config.pipe_src_h))
+ return 0;
+
+ if (((adjusted_mode-crtc_hdisplay != new_width) ||
+(adjusted_mode-crtc_vdisplay != new_height))
+ ((HAS_PCH_SPLIT(dev)
+ (!intel_crtc-config.pch_pfit.enabled)) ||
+(!HAS_PCH_SPLIT(dev)
+ (!intel_crtc-config.gmch_pfit.control {
+ DRM_ERROR(PIPESRC mismatch with Pipe timings PF is
disabled\n);
+ return -EINVAL;
+ }
+
+ intel_crtc-config.pipe_src_w = new_width;
+ intel_crtc-config.pipe_src_h = new_height;
+
+ intel_crtc-config.requested_mode.hdisplay = new_width;
+ intel_crtc-config.requested_mode.vdisplay = new_height;
+
+ crtc-mode.hdisplay = new_width;
+ crtc-mode.vdisplay = new_height;
+
+ /* pipesrc controls the size that is scaled from, which should
+ * always be the user's requested size.
+ */
+ I915_WRITE(PIPESRC(intel_crtc-pipe),
+ ((intel_crtc-config.pipe_src_w - 1) 16) |
+(intel_crtc-config.pipe_src_h - 1));
+
+ return 0;
+ }
+
return ret;
}
@@ -10586,6 +10638,15 @@ static void intel_crtc_init(struct drm_device *dev,
int pipe)
dev_priv-pipe_to_crtc_mapping[intel_crtc-pipe] = intel_crtc-base;
drm_crtc_helper_add(intel_crtc-base, intel_helper_funcs);
+
+ if (!dev_priv-input_size_property)
+
[Intel-gfx] [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
From: Akash Goel akash.g...@intel.com
This patch adds a new drm crtc property for varying the size of
the horizontal vertical borers of the output/display window.
This will control the output of Panel fitter.
v2: Added a new check for the invalid border size input
v3: Fixed bugs in output window calculation
Removed superfluous checks
v4: Added the capability to forecfully enable the Panel fitter.
The property value is of 64 bits, first 32 bits are used for
border dimensions. The 33rd bit can be used to forcefully
enable the panel fitter. This is useful for Panels which
do not override the User specified Pipe timings.
Testcase: igt/kms_panel_fitter_test
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++
drivers/gpu/drm/i915/intel_display.c | 39 +++-
drivers/gpu/drm/i915/intel_drv.h | 5 +
drivers/gpu/drm/i915/intel_panel.c | 176 ---
4 files changed, 211 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
*/
struct drm_property *input_size_property;
+ /*
+* Property to dynamically vary the size of the
+* borders. This will indirectly control the size
+* of the display window i.e Panel fitter output
+*/
+ struct drm_property *output_border_property;
+
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7149123..a217f25 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10447,7 +10447,23 @@ static int intel_crtc_set_property(struct drm_crtc
*crtc,
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int ret = -ENOENT;
+
+ if (property == dev_priv-output_border_property) {
+ if ((val == (uint64_t)intel_crtc-border_size)
+ (((val 32) 0x1) == intel_crtc-pfit_enabled))
+ return 0;
+
+ intel_crtc-border_size = (uint32_t)val;
+ intel_crtc-pfit_enabled = (val 32) 0x1;
+ if ((intel_crtc-border_size != 0)
+ (!intel_crtc-pfit_enabled)) {
+ DRM_ERROR(Wrong setting, expect Pfit to be enabled
when
+ applying borders\n);
+ return -EINVAL;
+ }
+
+ goto done;
+ }
if (property == dev_priv-input_size_property) {
int new_width = (int)((val 16) 0x);
@@ -10488,7 +10504,13 @@ static int intel_crtc_set_property(struct drm_crtc
*crtc,
return 0;
}
- return ret;
+ return -EINVAL;
+
+done:
+ if (crtc)
+ intel_crtc_restore_mode(crtc);
+
+ return 0;
}
static const struct drm_crtc_funcs intel_crtc_funcs = {
@@ -10641,12 +10663,23 @@ static void intel_crtc_init(struct drm_device *dev,
int pipe)
if (!dev_priv-input_size_property)
dev_priv-input_size_property =
- drm_property_create_range(dev, 0, input size, 0,
0x);
+ drm_property_create_range(dev, 0, input size,
+ 0, 0x);
if (dev_priv-input_size_property)
drm_object_attach_property(intel_crtc-base.base,
dev_priv-input_size_property,
0);
+
+ if (!dev_priv-output_border_property)
+ dev_priv-output_border_property =
+ drm_property_create_range(dev, 0, border size,
+ 0, (uint64_t)0x1);
+
+ if (dev_priv-output_border_property)
+ drm_object_attach_property(intel_crtc-base.base,
+ dev_priv-output_border_property,
+ 0);
}
enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..3cfc9da 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,11 @@ struct intel_crtc {
bool cpu_fifo_underrun_disabled;
bool pch_fifo_underrun_disabled;
+ /* for forceful enabling of panel fitter */
+ bool pfit_enabled;
+ /* border info for the output/display window */
+ uint32_t border_size;
+
/* per-pipe watermark state */
struct {
/* watermarks currently being
[Intel-gfx] [PATCH v2] tests/kms_panel_fitter: Test to verify the 2 new drm crtc properties
From: Akash Goel akash.g...@intel.com
This test is a derivative of kms_setmode. This will verify the 2 new drm
crtc properties, added to control the Panel fitter's input output.
v2: Modified the setting of 'border size' property. As now the 33rd bit
can be used to forcefully enable the Panel fitter.
Signed-off-by: Akash Goel akash.g...@intel.com
Tested-by: Akash Goel akash.g...@intel.com
---
tests/Makefile.sources |1 +
tests/kms_panel_fitter.c | 1215 ++
2 files changed, 1216 insertions(+)
create mode 100644 tests/kms_panel_fitter.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 88866ac..05ee06c 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -62,6 +62,7 @@ TESTS_progs_M = \
kms_plane \
kms_render \
kms_setmode \
+ kms_panel_fitter \
pm_lpsp \
pm_pc8 \
pm_rps \
diff --git a/tests/kms_panel_fitter.c b/tests/kms_panel_fitter.c
new file mode 100644
index 000..3134ba8
--- /dev/null
+++ b/tests/kms_panel_fitter.c
@@ -0,0 +1,1215 @@
+/*
+ * 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 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:
+ * ? @intel.com
+ */
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include assert.h
+#include cairo.h
+#include errno.h
+#include stdint.h
+#include unistd.h
+#include string.h
+#include getopt.h
+#include sys/time.h
+
+#include drm_fourcc.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_gpu_tools.h
+#include igt_kms.h
+#include igt_debugfs.h
+
+#define MAX_CONNECTORS 10
+#define MAX_CRTCS 3
+
+/* max combinations with repetitions */
+#define MAX_COMBINATION_COUNT \
+ (MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS)
+#define MAX_COMBINATION_ELEMS MAX_CRTCS
+
+static int drm_fd;
+static drmModeRes *drm_resources;
+static int filter_test_id;
+static bool dry_run;
+
+/*const*/ drmModeModeInfo mode_640_480 = {
+ .name = 640x480,
+ .vrefresh = 60,
+ .clock = 25200,
+
+ .hdisplay = 640,
+ .hsync_start= 656,
+ .hsync_end = 752,
+ .htotal = 800,
+
+ .vdisplay = 480,
+ .vsync_start= 490,
+ .vsync_end = 492,
+ .vtotal = 525,
+
+ .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+enum test_flags {
+ TEST_INVALID= 0x01,
+ TEST_CLONE = 0x02,
+ TEST_SINGLE_CRTC_CLONE = 0x04,
+ TEST_EXCLUSIVE_CRTC_CLONE = 0x08,
+ TEST_VARY_INPUT = 0x10,
+ TEST_VARY_BORDER= 0x20,
+};
+
+struct test_config {
+ const char *name;
+ enum test_flags flags;
+ drmModeRes *resources;
+};
+
+struct connector_config {
+ drmModeConnector *connector;
+ int crtc_idx;
+ bool connected;
+ drmModeModeInfo default_mode;
+};
+
+struct crtc_config {
+ int crtc_idx;
+ int crtc_id;
+ int pipe_id;
+ int connector_count;
+ struct connector_config *cconfs;
+ struct kmstest_fb fb_info;
+ drmModeModeInfo mode;
+};
+
+igt_debugfs_t debugfs;
+igt_pipe_crc_t **pipe_crc;
+static void init_crc(void)
+{
+ int j;
+ igt_debugfs_init(debugfs);
+ igt_pipe_crc_check(debugfs);
+ pipe_crc = calloc(2, sizeof(pipe_crc[0]));
+ for (j = 0; j 2; j++) {
+ int crtc_idx = j;
+ igt_pipe_crc_t *pipe_crc_p = igt_pipe_crc_new(debugfs, drm_fd,
crtc_idx,INTEL_PIPE_CRC_SOURCE_AUTO);
+ if (!pipe_crc_p) {
+ fprintf(stdout, auto crc not supported on this
connector with crtc %i\n,
+ crtc_idx);
+ return;
+ }
+ pipe_crc[crtc_idx] = pipe_crc_p;
+ }
+}
+
+static void read_crc(int crtc_idx)
+{
+ igt_pipe_crc_t *pipe_crc_cur =
[Intel-gfx] Command parser breaks the 3D driver.
Hello, The version of the command parser which landed in drm-intel-nightly (and is now enabled by default) completely breaks the 3D driver. Running any program - glxgears, KDE, GNOME, whatever - results in: intel_do_flush_locked failed: Invalid argument and then Mesa aborts the program. When Mesa initializes, it tries to submit several small batches to see if it can write various registers. For example: MI_LOAD_REGISTER_IMM | (3 - 2) OACONTROL 0x31337000 (expected value) various pipe controls MI_STORE_REGISTER_MEM OACONTROL address MI_LOAD_REGISTER_IMM | (3 - 2) OACONTROL 0 MI_BATCH_BUFFER_END We then map the buffer to see what the value is. If it's our expected value, we know we can write that register, and enable features. If not, we disable the functionality and never write that register again. This works because the hardware validator implicitly converts privileged commands (like MI_LOAD_REGISTER_IMM) to MI_NOOP, but otherwise accepts and processes the batch. This is well-documented behavior, and we've been relying on it since May 2013. In contrast, the software validator returns -EINVAL and skips executing the batch. It rejects this particular batch since OACONTROL is not in the kernel's register whitelist. I'm not sure I'm quite comfortable with the software validator implementing different behavior than the hardware validator. Then again, it's probably better behavior... Also, I'm surprised to see that the software validator is always enabled on Haswell. The hardware validator actually works on Haswell, and the majority of our batches don't need to run privileged commands, so it seems like we're just burning CPU pointlessly. I thought the plan was to have userspace add an execbuf flag to explicitly request software scanning when it emits privileged commands, and (on Haswell) use the hardware scanner normally. --Ken signature.asc Description: OpenPGP digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'
On Tue, 2014-03-25 at 10:59 +, Chris Wilson wrote: On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote: From: Akash Goel akash.g...@intel.com Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'. This workaround has to be applied before doing TLB Invalidation on render ring. In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI Store data commands. Without this, hardware cannot guarantee the command after the PIPE_CONTROL with TLB inv will not use the old TLB values. Note, that our command programming sequence already has multiple dword writes between the flush of the last batch and the invalidation of the next. Is this w/a still required? Why? -Chris Hi Chris, Are you referring to the MI_STORE_DWORD_INDEX commands emitted as a part of add_request functions? We couldn't find any other place where we are storing dwords between flush of last batch and invalidation of next. In that case, we agree that as a part of command programming sequence, we'll have one set of store dwords emitted. But, as per spec, it is required to emit 2 sets of store dwords before the tlb invalidation. Also, our motive for having this w/a is just being paranoid, and not assuming that dwords would already have been emitted before doing tlb invalidation. So, we try to explicitly ensure the same through our w/a. Regards, Sourab ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.
Mesa needs to be able to write OACONTROL in order to expose the
Observability Architecture's performance counters via OpenGL.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 1 +
drivers/gpu/drm/i915/i915_reg.h| 2 ++
2 files changed, 3 insertions(+)
This patch needs to go before
commit 6d42f94084b8c69813d7ecd0466c33fe561bf127
Author: Brad Volkin bradley.d.vol...@intel.com
Date: Tue Feb 18 10:15:57 2014 -0800
drm/i915: Enable command parsing by default
in whatever branch gets submitted to Dave Airlie. Or, that commit needs
to be reverted. Otherwise, every OpenGL program will abort. Examples
of programs that abort include GNOME, KDE, Firefox, and glxgears.
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index bae7c2f..d4a50b9 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -415,6 +415,7 @@ static const u32 gen7_render_regs[] = {
GEN7_SO_WRITE_OFFSET(1),
GEN7_SO_WRITE_OFFSET(2),
GEN7_SO_WRITE_OFFSET(3),
+ OACONTROL,
};
static const u32 gen7_blt_regs[] = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9f9e2b7..0ebc20d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -427,6 +427,8 @@
/* There are the 4 64-bit counter registers, one for each stream output */
#define GEN7_SO_NUM_PRIMS_WRITTEN(n) (0x5200 + (n) * 8)
+#define OACONTROL 0x2360
+
#define _GEN7_PIPEA_DE_LOAD_SL 0x70068
#define _GEN7_PIPEB_DE_LOAD_SL 0x71068
#define GEN7_PIPE_DE_LOAD_SL(pipe) _PIPE(pipe, \
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
