[Intel-gfx] [PATCH V3] drm/i915: Add sprite watermark programming for VLV and CHV
Program DDL register as part of sprite watermark programming for CHV and VLV.
v2: Rename DRAIN_LATENCY_MAX by DRAIN_LATENCY_MASK
v3: Addressed review comments by Ville
- Changed Sprite DDL definitions to more generic to avoid multiple if-else
- Changed bit masking to customary form
- Changed to bitwise shorthand operator for sprite_dl assignment
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h |9 +++--
drivers/gpu/drm/i915/intel_pm.c | 33 +
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 08916df..f8aaf0b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4000,12 +4000,9 @@ enum punit_power_well {
#define DDL_CURSOR_PRECISION_64(131)
#define DDL_CURSOR_PRECISION_32(031)
#define DDL_CURSOR_SHIFT 24
-#define DDL_SPRITE1_PRECISION_64 (123)
-#define DDL_SPRITE1_PRECISION_32 (023)
-#define DDL_SPRITE1_SHIFT 16
-#define DDL_SPRITE0_PRECISION_64 (115)
-#define DDL_SPRITE0_PRECISION_32 (015)
-#define DDL_SPRITE0_SHIFT 8
+#define DDL_SPRITE_PRECISION_64(sprite)(1(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_32(sprite)(0(15+8*(sprite)))
+#define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
#define DDL_PLANE_PRECISION_64 (17)
#define DDL_PLANE_PRECISION_32 (07)
#define DDL_PLANE_SHIFT0
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 86d6048..974aeea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1500,6 +1500,37 @@ static void cherryview_update_wm(struct drm_crtc *crtc)
intel_set_memory_cxsr(dev_priv, true);
}
+static void valleyview_update_sprite_wm(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
+ bool enabled, bool scaled)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int pipe = to_intel_plane(plane)-pipe;
+ int sprite = to_intel_plane(plane)-plane;
+ int drain_latency;
+ int plane_prec;
+ int sprite_dl;
+ int prec_mult;
+
+ sprite_dl = I915_READ(VLV_DDL(pipe))
~(DDL_SPRITE_PRECISION_64(sprite) |
+ (DRAIN_LATENCY_MASK DDL_SPRITE_SHIFT(sprite)));
+
+ if (enabled vlv_compute_drain_latency(crtc, pixel_size, prec_mult,
+drain_latency)) {
+ plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+ DDL_SPRITE_PRECISION_64(sprite) :
+ DDL_SPRITE_PRECISION_32(sprite);
+ sprite_dl |= plane_prec |
+(drain_latency DDL_SPRITE_SHIFT(sprite));
+ }
+
+ I915_WRITE(VLV_DDL(pipe), sprite_dl);
+}
+
static void g4x_update_wm(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -7231,10 +7262,12 @@ void intel_init_pm(struct drm_device *dev)
dev_priv-display.init_clock_gating =
gen8_init_clock_gating;
} else if (IS_CHERRYVIEW(dev)) {
dev_priv-display.update_wm = cherryview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
cherryview_init_clock_gating;
} else if (IS_VALLEYVIEW(dev)) {
dev_priv-display.update_wm = valleyview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
valleyview_init_clock_gating;
} else if (IS_PINEVIEW(dev)) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] tests/gem_exec_params: check the invalid flags for dual bsd ring
Signed-off-by: Zhipeng Gong zhipeng.g...@intel.com
---
tests/gem_exec_params.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
index 769969d..b18ef4e 100644
--- a/tests/gem_exec_params.c
+++ b/tests/gem_exec_params.c
@@ -126,6 +126,12 @@ igt_main
RUN_FAIL(EINVAL);
}
+ igt_subtest(invalid-bsd-ring) {
+ igt_require(gem_has_bsd2(fd));
+ execbuf.flags = I915_EXEC_BSD | I915_EXEC_BSD_MASK;
+ RUN_FAIL(EINVAL);
+ }
+
igt_subtest(rel-constants-invalid-ring) {
igt_require(gem_has_bsd(fd));
execbuf.flags = I915_EXEC_BSD | I915_EXEC_CONSTANTS_ABSOLUTE;
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 0/3] tests: Add tests for new exec flags to export two bsd rings
The Broadwell GT3 machine has two independent BSD rings that can be used to process the video commands. New execution flags are added to export the two rings to userspace so that userspace can specify the bsd ring. Zhipeng Gong (3): i-g-t: check whether kernel has dual bsd ring tests/gem_exec_params: check the invalid flags for dual bsd ring test/gem_dummy_reloc_loop: add tests for dual bsd ring lib/ioctl_wrappers.c | 16 lib/ioctl_wrappers.h | 1 + tests/gem_dummy_reloc_loop.c | 16 tests/gem_exec_params.c | 6 ++ 4 files changed, 39 insertions(+) -- 2.0.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] i-g-t: check whether kernel has dual bsd ring
Signed-off-by: Zhipeng Gong zhipeng.g...@intel.com
---
lib/ioctl_wrappers.c | 16
lib/ioctl_wrappers.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index c4e1080..03163a0 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -754,6 +754,22 @@ bool gem_has_vebox(int fd)
return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_VEBOX);
}
+#define LOCAL_I915_PARAM_HAS_BSD2 29
+/**
+ * gem_has_bsd2:
+ * @fd: open i915 drm file descriptor
+ *
+ * Feature test macro to query whether the BSD2 ring is available. This is
simply
+ * a specific version of gem_has_enable_ring() for the BSD2 ring.
+ *
+ * Note that recent Bspec calls this the VCS ring for Video Command Submission.
+ *
+ * Returns: Whether the BSD ring is avaible or not.
+ */
+bool gem_has_bsd2(int fd)
+{
+ return gem_has_enable_ring(fd,I915_PARAM_HAS_BSD2);
+}
/**
* gem_available_aperture_size:
* @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 310d82e..2979634 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -83,6 +83,7 @@ bool gem_has_enable_ring(int fd,int param);
bool gem_has_bsd(int fd);
bool gem_has_blt(int fd);
bool gem_has_vebox(int fd);
+bool gem_has_bsd2(int fd);
bool gem_uses_aliasing_ppgtt(int fd);
int gem_available_fences(int fd);
uint64_t gem_available_aperture_size(int fd);
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] test/gem_dummy_reloc_loop: add tests for dual bsd ring
Signed-off-by: Zhipeng Gong zhipeng.g...@intel.com
---
tests/gem_dummy_reloc_loop.c | 16
1 file changed, 16 insertions(+)
diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
index 4fe0786..6913537 100644
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -265,6 +265,22 @@ igt_main
}
#endif
+ igt_subtest(bsd-ring1) {
+ igt_require(gem_has_bsd2(fd));
+ sleep(2);
+ igt_info(running dummy loop on bsd-ring1\n);
+ dummy_reloc_loop(I915_EXEC_BSD|I915_EXEC_BSD_RING1);
+ igt_info(dummy loop run on bsd-ring1 completed\n);
+ }
+
+ igt_subtest(bsd-ring2) {
+ igt_require(gem_has_bsd2(fd));
+ sleep(2);
+ igt_info(running dummy loop on bsd-ring2\n);
+ dummy_reloc_loop(I915_EXEC_BSD|I915_EXEC_BSD_RING2);
+ igt_info(dummy loop run on bsd-ring2 completed\n);
+ }
+
igt_subtest(mixed) {
if (num_rings 1) {
sleep(2);
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Specify bsd rings through exec flag
On Broadwell GT3 we have 2 Video Command Streamers (VCS), but userspace
has no control when using VCS1 or VCS2. This patch introduces a mechanism
to avoid the default ping-pong mode and use one specific ring through
execution flag.
Signed-off-by: Zhipeng Gong zhipeng.g...@intel.com
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +--
include/uapi/drm/i915_drm.h| 8 +++-
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 60998fc..f9ed8e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1279,8 +1279,23 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
else if ((args-flags I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
if (HAS_BSD2(dev)) {
int ring_id;
- ring_id = gen8_dispatch_bsd_ring(dev, file);
- ring = dev_priv-ring[ring_id];
+
+ switch (args-flags I915_EXEC_BSD_MASK) {
+ case I915_EXEC_BSD_DEFAULT:
+ ring_id = gen8_dispatch_bsd_ring(dev, file);
+ ring = dev_priv-ring[ring_id];
+ break;
+ case I915_EXEC_BSD_RING1:
+ ring = dev_priv-ring[VCS];
+ break;
+ case I915_EXEC_BSD_RING2:
+ ring = dev_priv-ring[VCS2];
+ break;
+ default:
+ DRM_DEBUG(execbuf with unknown bsd ring: %d\n,
+ (int)(args-flags
I915_EXEC_BSD_MASK));
+ return -EINVAL;
+ }
} else
ring = dev_priv-ring[VCS];
} else
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ff57f07..421420a 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -736,7 +736,13 @@ struct drm_i915_gem_execbuffer2 {
*/
#define I915_EXEC_HANDLE_LUT (112)
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT1)
+/** Used for switching BSD rings on the platforms with two BSD rings */
+#define I915_EXEC_BSD_MASK (313)
+#define I915_EXEC_BSD_DEFAULT (013) /* default ping-pong mode */
+#define I915_EXEC_BSD_RING1(113)
+#define I915_EXEC_BSD_RING2(213)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(115)
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam
This will let userland only try to use the new ring
when the appropriate kernel is present
Signed-off-by: Zhipeng Gong zhipeng.g...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 3 +++
include/uapi/drm/i915_drm.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03a..ce217e6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -977,6 +977,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_VEBOX:
value = intel_ring_initialized(dev_priv-ring[VECS]);
break;
+ case I915_PARAM_HAS_BSD2:
+ value = intel_ring_initialized(dev_priv-ring[VCS2]);
+ break;
case I915_PARAM_HAS_RELAXED_FENCING:
value = 1;
break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 421420a..0518705 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -340,6 +340,7 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_HAS_EXEC_HANDLE_LUT 26
#define I915_PARAM_HAS_WT 27
#define I915_PARAM_CMD_PARSER_VERSION 28
+#define I915_PARAM_HAS_BSD2 29
typedef struct drm_i915_getparam {
int param;
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V3] drm/i915: Add sprite watermark programming for VLV and CHV
On Thu, Aug 07, 2014 at 05:03:30PM +0530, Gajanan Bhat wrote:
Program DDL register as part of sprite watermark programming for CHV and VLV.
v2: Rename DRAIN_LATENCY_MAX by DRAIN_LATENCY_MASK
v3: Addressed review comments by Ville
- Changed Sprite DDL definitions to more generic to avoid multiple if-else
- Changed bit masking to customary form
- Changed to bitwise shorthand operator for sprite_dl assignment
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Looks good.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h |9 +++--
drivers/gpu/drm/i915/intel_pm.c | 33 +
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 08916df..f8aaf0b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4000,12 +4000,9 @@ enum punit_power_well {
#define DDL_CURSOR_PRECISION_64 (131)
#define DDL_CURSOR_PRECISION_32 (031)
#define DDL_CURSOR_SHIFT 24
-#define DDL_SPRITE1_PRECISION_64 (123)
-#define DDL_SPRITE1_PRECISION_32 (023)
-#define DDL_SPRITE1_SHIFT16
-#define DDL_SPRITE0_PRECISION_64 (115)
-#define DDL_SPRITE0_PRECISION_32 (015)
-#define DDL_SPRITE0_SHIFT8
+#define DDL_SPRITE_PRECISION_64(sprite) (1(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_32(sprite) (0(15+8*(sprite)))
+#define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
#define DDL_PLANE_PRECISION_64 (17)
#define DDL_PLANE_PRECISION_32 (07)
#define DDL_PLANE_SHIFT 0
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 86d6048..974aeea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1500,6 +1500,37 @@ static void cherryview_update_wm(struct drm_crtc *crtc)
intel_set_memory_cxsr(dev_priv, true);
}
+static void valleyview_update_sprite_wm(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
+ bool enabled, bool scaled)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int pipe = to_intel_plane(plane)-pipe;
+ int sprite = to_intel_plane(plane)-plane;
+ int drain_latency;
+ int plane_prec;
+ int sprite_dl;
+ int prec_mult;
+
+ sprite_dl = I915_READ(VLV_DDL(pipe))
~(DDL_SPRITE_PRECISION_64(sprite) |
+ (DRAIN_LATENCY_MASK DDL_SPRITE_SHIFT(sprite)));
+
+ if (enabled vlv_compute_drain_latency(crtc, pixel_size, prec_mult,
+ drain_latency)) {
+ plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+DDL_SPRITE_PRECISION_64(sprite) :
+DDL_SPRITE_PRECISION_32(sprite);
+ sprite_dl |= plane_prec |
+ (drain_latency DDL_SPRITE_SHIFT(sprite));
+ }
+
+ I915_WRITE(VLV_DDL(pipe), sprite_dl);
+}
+
static void g4x_update_wm(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -7231,10 +7262,12 @@ void intel_init_pm(struct drm_device *dev)
dev_priv-display.init_clock_gating =
gen8_init_clock_gating;
} else if (IS_CHERRYVIEW(dev)) {
dev_priv-display.update_wm = cherryview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
cherryview_init_clock_gating;
} else if (IS_VALLEYVIEW(dev)) {
dev_priv-display.update_wm = valleyview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
valleyview_init_clock_gating;
} else if (IS_PINEVIEW(dev)) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV
On Thu, 2014-08-07 at 11:26 +0530, Bhat, Gajanan wrote:
On 7/31/2014 7:14 PM, Imre Deak wrote:
On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
Program DDL register as part sprite watermark programming for CHV and VLV.
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
This looks ok, but could you confirm, ideally referencing some document,
that we don't need to program any of the sprite watermark level
registers along with the DDL values? Specifically I mean the FW7, FW8
registers.
I have looked at the B-spec to again confirm my understanding. The
chicken bit in CBR1 specifies which mechanism (PND or watermark) to be
used. In our case we are using only the PND with DDL. I couldn't find
any reference which explicitly tells that both DDL and watermarks
should be programmed together. Also we have tested this code and so far
it has not caused any problem.
Thanks for clarifying this, the docs on this are fuzzy to me. Earlier
Cesar and Ville also pointed out that the PND mechanism depends on the
MI_ARB[trickle feed] flag being enabled, which is afaics disabled on
VLV. But based on what you say I understand that the DDL values are not
used in the 'watermark mode', so I'm ok with your change. Perhaps a
comment about this could be added.
--Imre
-Gajanan
---
drivers/gpu/drm/i915/intel_pm.c | 44
+++
1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index f3a3e90..0f439f7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc
*crtc)
intel_set_memory_cxsr(dev_priv, true);
}
+static void valleyview_update_sprite_wm(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
+ bool enabled, bool scaled)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int pipe = to_intel_plane(plane)-pipe;
+ int drain_latency;
+ int plane_prec;
+ int sprite_dl;
+ int prec_mult;
+
+ if (to_intel_plane(plane)-plane == 0)
+ sprite_dl = I915_READ(VLV_DDL(pipe))
~DDL_SPRITE0_PRECISION_64
+ ~(DRAIN_LATENCY_MAX DDL_SPRITE0_SHIFT);
+ else
+ sprite_dl = I915_READ(VLV_DDL(pipe))
~DDL_SPRITE1_PRECISION_64
+ ~(DRAIN_LATENCY_MAX DDL_SPRITE1_SHIFT);
+
+ if (enabled vlv_compute_drain_latency(crtc, pixel_size, prec_mult,
+ drain_latency)) {
+ if (to_intel_plane(plane)-plane == 0) {
+ plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+ DDL_SPRITE0_PRECISION_64 :
+ DDL_SPRITE0_PRECISION_32;
+ sprite_dl = sprite_dl | plane_prec |
+ drain_latency DDL_SPRITE0_SHIFT;
+ } else {
+ plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+ DDL_SPRITE1_PRECISION_64 :
+ DDL_SPRITE1_PRECISION_32;
+ sprite_dl = sprite_dl | plane_prec |
+ drain_latency DDL_SPRITE1_SHIFT;
+ }
+ }
+
+ I915_WRITE(VLV_DDL(pipe), sprite_dl);
+}
+
static void g4x_update_wm(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev)
dev_priv-display.init_clock_gating =
gen8_init_clock_gating;
} else if (IS_CHERRYVIEW(dev)) {
dev_priv-display.update_wm = valleyview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
cherryview_init_clock_gating;
} else if (IS_VALLEYVIEW(dev)) {
dev_priv-display.update_wm = valleyview_update_wm;
+ dev_priv-display.update_sprite_wm =
valleyview_update_sprite_wm;
dev_priv-display.init_clock_gating =
valleyview_init_clock_gating;
} else if (IS_PINEVIEW(dev)) {
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
Re: [Intel-gfx] [PATCH 0/6] Rename DP training vswing/pre-emph defines
On Tue, Aug 05, 2014 at 04:38:16PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP 1.4 where the values are different. Updated in all the drivers as well Hi Sonika, Oops, another mess in this series :) If there's a need to rework the series and add/remove patches, the best way to send a v2 is to actually resend the whole series, otherwise someone will get it wrong and won't apply the right patches. So, could you resend the whole series again with the patches generated by Daniel's review comment and with the explanation copy/pasted in all the driver patches (as Jingoo Han asked). It's fair enough to track why the rename was needed in driver-specific patches instead of relying on the cover letter. Thanks, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Rename defines for selection of ddi buffer translation slot
On Tue, Aug 05, 2014 at 05:36:19PM +0530, sonika.jin...@intel.com wrote:
From: Sonika Jindal sonika.jin...@intel.com
Renaming the HSW-specific macros for ddi buffer translation slot to denote the
slot and not the vswing/pre-emph values as they are platform-dependent.
This patch is based on top of the patch series for renaming the DP training
vswing/pre-emph defines:
http://lists.freedesktop.org/archives/intel-gfx/2014-August/050123.html
While this does look correct, I think we can simplify the code a bit
more by defining:
#define DDI_BUF_TRANS_SELECT(n) ((n) 24)
This will allow us to remove the hsw_ddi_buf_ctl_values array by being
able to directy address the entry in the FDI training code (with
something like DDI_BUF_TRANS_SELECT(i / 2) in the current code).
Of course the ARRAY_SIZE(hsw_ddi_buf_ctl_values) will also have to be
changed, maybe by adding a define just above hsw_fdi_link_train() with
the number of translation entries, for instance:
#define NUM_FDI_TRANSLATION_ENTRIES (ARRAY_SIZE(hsw_ddi_translations_fdi) / 2)
--
Damien
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 18 +-
drivers/gpu/drm/i915/intel_ddi.c | 20 ++--
drivers/gpu/drm/i915/intel_dp.c | 20 ++--
3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc13961..81c6b51 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5888,15 +5888,15 @@ enum punit_power_well {
#define DDI_BUF_CTL_B0x64100
#define DDI_BUF_CTL(port) _PORT(port, DDI_BUF_CTL_A, DDI_BUF_CTL_B)
#define DDI_BUF_CTL_ENABLE (131)
-#define DDI_BUF_EMP_400MV_0DB_HSW (024) /* Sel0 */
-#define DDI_BUF_EMP_400MV_3_5DB_HSW (124) /* Sel1 */
-#define DDI_BUF_EMP_400MV_6DB_HSW (224) /* Sel2 */
-#define DDI_BUF_EMP_400MV_9_5DB_HSW (324) /* Sel3 */
-#define DDI_BUF_EMP_600MV_0DB_HSW (424) /* Sel4 */
-#define DDI_BUF_EMP_600MV_3_5DB_HSW (524) /* Sel5 */
-#define DDI_BUF_EMP_600MV_6DB_HSW (624) /* Sel6 */
-#define DDI_BUF_EMP_800MV_0DB_HSW (724) /* Sel7 */
-#define DDI_BUF_EMP_800MV_3_5DB_HSW (824) /* Sel8 */
+#define DDI_BUF_TRANS_SELECT_0 (024)
+#define DDI_BUF_TRANS_SELECT_1 (124)
+#define DDI_BUF_TRANS_SELECT_2 (224)
+#define DDI_BUF_TRANS_SELECT_3 (324)
+#define DDI_BUF_TRANS_SELECT_4 (424)
+#define DDI_BUF_TRANS_SELECT_5 (524)
+#define DDI_BUF_TRANS_SELECT_6 (624)
+#define DDI_BUF_TRANS_SELECT_7 (724)
+#define DDI_BUF_TRANS_SELECT_8 (824)
#define DDI_BUF_EMP_MASK(0xf24)
#define DDI_BUF_PORT_REVERSAL (116)
#define DDI_BUF_IS_IDLE (17)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index ca1f9a8..a616747 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -242,15 +242,15 @@ void intel_prepare_ddi(struct drm_device *dev)
}
static const long hsw_ddi_buf_ctl_values[] = {
- DDI_BUF_EMP_400MV_0DB_HSW,
- DDI_BUF_EMP_400MV_3_5DB_HSW,
- DDI_BUF_EMP_400MV_6DB_HSW,
- DDI_BUF_EMP_400MV_9_5DB_HSW,
- DDI_BUF_EMP_600MV_0DB_HSW,
- DDI_BUF_EMP_600MV_3_5DB_HSW,
- DDI_BUF_EMP_600MV_6DB_HSW,
- DDI_BUF_EMP_800MV_0DB_HSW,
- DDI_BUF_EMP_800MV_3_5DB_HSW
+ DDI_BUF_TRANS_SELECT_0,
+ DDI_BUF_TRANS_SELECT_1,
+ DDI_BUF_TRANS_SELECT_2,
+ DDI_BUF_TRANS_SELECT_3,
+ DDI_BUF_TRANS_SELECT_4,
+ DDI_BUF_TRANS_SELECT_5,
+ DDI_BUF_TRANS_SELECT_6,
+ DDI_BUF_TRANS_SELECT_7,
+ DDI_BUF_TRANS_SELECT_8
};
static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
@@ -402,7 +402,7 @@ void intel_ddi_init_dp_buf_reg(struct intel_encoder
*encoder)
enc_to_dig_port(encoder-base);
intel_dp-DP = intel_dig_port-saved_port_bits |
- DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
+ DDI_BUF_CTL_ENABLE | DDI_BUF_TRANS_SELECT_0;
intel_dp-DP |= DDI_PORT_WIDTH(intel_dp-lane_count);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c2b3075..7a8ee50 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2854,29 +2854,29 @@ intel_hsw_signal_levels(uint8_t train_set)
DP_TRAIN_PRE_EMPHASIS_MASK);
switch (signal_levels) {
case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | DP_TRAIN_PRE_EMPHASIS_LEVEL_0:
- return DDI_BUF_EMP_400MV_0DB_HSW;
+ return DDI_BUF_TRANS_SELECT_0;
case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
+ /* FBC does not work on some platforms for rotated planes */
+ if (INTEL_INFO(dev)-gen = 4 !IS_G4X(dev)) {
+ if (dev_priv-fbc.plane == intel_crtc-plane
+ intel_plane-rotation != BIT(DRM_ROTATE_0))
+ intel_disable_fbc(dev);
+ /* If rotation was set earlier and new rotation is 0,
+ we might have disabled fbc earlier. So update it now */
+ else if (intel_plane-rotation ==
BIT(DRM_ROTATE_0)
+old_val != BIT(DRM_ROTATE_0)) {
+ mutex_lock(dev-struct_mutex);
+ intel_update_fbc(dev);
+ mutex_unlock(dev-struct_mutex);
+ }
+ }
Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.
Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.
Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.
I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
to call the update_plane function (not the raw low-level one, the
high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
correctness with screen rotation. Probably best to reuse the existing
fbc testcase and add a bunch or rotated tests.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Add 180 degree primary plane rotation support
On Thu, Aug 07, 2014 at 01:45:31PM +0200, Daniel Vetter wrote:
On Tue, Jul 15, 2014 at 11:11:37AM +0200, Daniel Vetter wrote:
On Tue, Jul 15, 2014 at 02:10:28PM +0530, sonika.jin...@intel.com wrote:
+ /* FBC does not work on some platforms for rotated planes */
+ if (INTEL_INFO(dev)-gen = 4 !IS_G4X(dev)) {
+ if (dev_priv-fbc.plane == intel_crtc-plane
+ intel_plane-rotation != BIT(DRM_ROTATE_0))
+ intel_disable_fbc(dev);
+ /* If rotation was set earlier and new rotation is 0,
+ we might have disabled fbc earlier. So update it now */
+ else if (intel_plane-rotation ==
BIT(DRM_ROTATE_0)
+ old_val != BIT(DRM_ROTATE_0)) {
+ mutex_lock(dev-struct_mutex);
+ intel_update_fbc(dev);
+ mutex_unlock(dev-struct_mutex);
+ }
+ }
Indentation is screwed up here. Also if we convert some of the checks into
early bails we could de-indent this by one level.
Also Chris mentioned that on some platforms this won't work and it's more
future-proof to just do a full modeset until we have the proper
infrastructure.
Apparently this review here was never addressed, as Chris just pointed out
on irc. I've dropped the patch again.
I think we need:
- The same sequence as with the sprite set_property function, i.e. we need
to call the update_plane function (not the raw low-level one, the
high-level with all the checks).
- The fbc check is wrong and will miss updates when the crtc is off. We
need to move this into the general list of checks in intel_update_fbc.
- Since this seems to be buggy I want added testcases to combine fbc
correctness with screen rotation. Probably best to reuse the existing
fbc testcase and add a bunch or rotated tests.
Ok, the check in update_fbc is there, I've been blind. Sorry about all the
confusion. So just amounts of calling the higher level function and we can
forgo the fbc testing.
-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 12/43] drm/i915/bdw: Don't write PDP in the legacy way when using LRCs
From: Oscar Mateo oscar.ma...@intel.com
This is mostly for correctness so that we know we are running the LR
context correctly (this is, the PDPs are contained inside the context
object).
v2: Move the check to inside the enable PPGTT function. The switch
happens in two places: the legacy context switch (that we won't hit
when Execlists are enabled) and the PPGTT enable, which unfortunately
we need. This would look much nicer if the ppgtt-enable was part of
the ring init, where it logically belongs.
v3: Move the check to the start of the enable PPGTT function. None
of the legacy PPGTT enabling is required when using LRCs as the
PPGTT is enabled in the context descriptor and the PDPs are written
in the LRC.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5188936..cfbf272 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -843,6 +843,11 @@ static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
struct intel_engine_cs *ring;
int j, ret;
+ /* In the case of Execlists, we don't want to write the PDPs
+* in the legacy way (they live inside the context now) */
+ if (i915.enable_execlists)
+ return 0;
+
for_each_ring(ring, dev_priv, j) {
I915_WRITE(RING_MODE_GEN7(ring),
_MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 37/43] drm/i915/bdw: Display execlists info in debugfs
From: Oscar Mateo oscar.ma...@intel.com
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
v4: Lock the struct mutex for atomic state capture
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 80 +++
drivers/gpu/drm/i915/intel_lrc.c|6 ---
drivers/gpu/drm/i915/intel_lrc.h|7 +++
3 files changed, 87 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index fc39610..f8f0e11 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1674,6 +1674,85 @@ static int i915_context_status(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_execlists(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_engine_cs *ring;
+ u32 status_pointer;
+ u8 read_pointer;
+ u8 write_pointer;
+ u32 status;
+ u32 ctx_id;
+ struct list_head *cursor;
+ int ring_id, i;
+ int ret;
+
+ if (!i915.enable_execlists) {
+ seq_printf(m, Logical Ring Contexts are disabled\n);
+ return 0;
+ }
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ for_each_ring(ring, dev_priv, ring_id) {
+ struct intel_ctx_submit_request *head_req = NULL;
+ int count = 0;
+ unsigned long flags;
+
+ seq_printf(m, %s\n, ring-name);
+
+ status = I915_READ(RING_EXECLIST_STATUS(ring));
+ ctx_id = I915_READ(RING_EXECLIST_STATUS(ring) + 4);
+ seq_printf(m, \tExeclist status: 0x%08X, context: %u\n,
+ status, ctx_id);
+
+ status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
+ seq_printf(m, \tStatus pointer: 0x%08X\n, status_pointer);
+
+ read_pointer = ring-next_context_status_buffer;
+ write_pointer = status_pointer 0x07;
+ if (read_pointer write_pointer)
+ write_pointer += 6;
+ seq_printf(m, \tRead pointer: 0x%08X, write pointer 0x%08X\n,
+ read_pointer, write_pointer);
+
+ for (i = 0; i 6; i++) {
+ status = I915_READ(RING_CONTEXT_STATUS_BUF(ring) + 8*i);
+ ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) + 8*i
+ 4);
+
+ seq_printf(m, \tStatus buffer %d: 0x%08X, context:
%u\n,
+ i, status, ctx_id);
+ }
+
+ spin_lock_irqsave(ring-execlist_lock, flags);
+ list_for_each(cursor, ring-execlist_queue)
+ count++;
+ head_req = list_first_entry_or_null(ring-execlist_queue,
+ struct intel_ctx_submit_request, execlist_link);
+ spin_unlock_irqrestore(ring-execlist_lock, flags);
+
+ seq_printf(m, \t%d requests in queue\n, count);
+ if (head_req) {
+ struct drm_i915_gem_object *ctx_obj;
+
+ ctx_obj = head_req-ctx-engine[ring_id].state;
+ seq_printf(m, \tHead request id: %u\n,
+ intel_execlists_ctx_id(ctx_obj));
+ seq_printf(m, \tHead request tail: %u\n,
head_req-tail);
+ }
+
+ seq_putc(m, '\n');
+ }
+
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m-private;
@@ -3899,6 +3978,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{i915_opregion, i915_opregion, 0},
{i915_gem_framebuffer, i915_gem_framebuffer_info, 0},
{i915_context_status, i915_context_status, 0},
+ {i915_execlists, i915_execlists, 0},
{i915_gen6_forcewake_count, i915_gen6_forcewake_count_info, 0},
{i915_swizzle_info, i915_swizzle_info, 0},
{i915_ppgtt_info, i915_ppgtt_info, 0},
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 829b15d..8056fa4 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -46,12 +46,6 @@
#define GEN8_LR_CONTEXT_ALIGN 4096
-#define RING_ELSP(ring)((ring)-mmio_base+0x230)
-#define RING_EXECLIST_STATUS(ring) ((ring)-mmio_base+0x234)
-#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
-#define RING_CONTEXT_STATUS_BUF(ring) ((ring)-mmio_base+0x370)
[Intel-gfx] [PATCH 39/43] drm/i915/bdw: Print context state in debugfs
From: Ben Widawsky b...@bwidawsk.net
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error
state. I've used it quite a bit in debugfs however, and I'd like to keep
it here at least until preemption is working.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
This patch was accidentally dropped in the first Execlists version, and
it has been very useful indeed. Put it back again, but as a standalone
debugfs file.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
v2: Take the device struct_mutex rather than mode_config mutex for
atomic state capture.
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 52 +++
1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index aca5ff1..a3c958c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_dump_lrc(struct seq_file *m, void *unused)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_engine_cs *ring;
+ struct intel_context *ctx;
+ int ret, i;
+
+ if (!i915.enable_execlists) {
+ seq_printf(m, Logical Ring Contexts are disabled\n);
+ return 0;
+ }
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(ctx, dev_priv-context_list, link) {
+ for_each_ring(ring, dev_priv, i) {
+ struct drm_i915_gem_object *ctx_obj =
ctx-engine[i].state;
+
+ if (ring-default_context == ctx)
+ continue;
+
+ if (ctx_obj) {
+ struct page *page =
i915_gem_object_get_page(ctx_obj, 1);
+ uint32_t *reg_state = kmap_atomic(page);
+ int j;
+
+ seq_printf(m, CONTEXT: %s %u\n, ring-name,
+
intel_execlists_ctx_id(ctx_obj));
+
+ for (j = 0; j 0x600 / sizeof(u32) / 4; j +=
4) {
+ seq_printf(m, \t[0x%08lx] 0x%08x
0x%08x 0x%08x 0x%08x\n,
+ i915_gem_obj_ggtt_offset(ctx_obj) +
4096 + (j * 4),
+ reg_state[j], reg_state[j + 1],
+ reg_state[j + 2], reg_state[j + 3]);
+ }
+ kunmap_atomic(reg_state);
+
+ seq_putc(m, '\n');
+ }
+ }
+ }
+
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static int i915_execlists(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -3999,6 +4050,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{i915_opregion, i915_opregion, 0},
{i915_gem_framebuffer, i915_gem_framebuffer_info, 0},
{i915_context_status, i915_context_status, 0},
+ {i915_dump_lrc, i915_dump_lrc, 0},
{i915_execlists, i915_execlists, 0},
{i915_gen6_forcewake_count, i915_gen6_forcewake_count_info, 0},
{i915_swizzle_info, i915_swizzle_info, 0},
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc: regression in suspend
On Fri, 11 Jul 2014, Pavel Machek wrote: Ok, so I have set up machines for ktest / autobisect, and found out that 3.16-rc1 no longer has that problem. Oh well, bisect would not be fun, anyway... I am still seeing the problem with 3.16-rc2. I'm confused now. Is the bisect result commit 773875bfb6737982903c42d1ee88cf60af80089c Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Jan 27 10:00:30 2014 +0100 drm/i915: Don't set the 8to6 dither flag when not scaling now the culprit or not? Or do we have 2 different bugs at hand here? Three different issues, it seems. Two ring initialization problems, one went away in 3.16 (for me), second did not (suspend for jikos), third -- trivial issue with 8to6 dither. The patch below seems to finally cure the problem at my system; I've just attached it to freedesktop bugzilla, but sending it to this thread as well to hopefully get as much testing coverage by affected people as possible. I am going on with testing whether it really completely fixes the problem or just made it less likely. From: Jiri Kosina jkos...@suse.cz Subject: [PATCH] drm/i915: read HEAD register back in init_ring_common() to enforce ordering Withtout this, ring initialization fails reliabily during resume with [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head ff8804 tail start 000e4000 Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..7add7ee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -517,6 +517,9 @@ static int init_ring_common(struct intel_engine_cs *ring) else ring_setup_phys_status_page(ring); + /* Enforce ordering by reading HEAD register back */ + I915_READ_HEAD(ring); + /* Initialize the ring. This must happen _after_ we've cleared the ring * registers with the above sequence (the readback of the HEAD registers * also enforces ordering), otherwise the hw might lose the new ring -- Jiri Kosina SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc: regression in suspend
On Thu, 7 Aug 2014, Jiri Kosina wrote: The patch below seems to finally cure the problem at my system; I've just attached it to freedesktop bugzilla, but sending it to this thread as well to hopefully get as much testing coverage by affected people as possible. I am going on with testing whether it really completely fixes the problem or just made it less likely. Okay, after 31 suspend-resume cycles, the problem appeared again (while without the patch, it triggers with 100% reliability). So it's not a complete fix, it just makes the problem much less visible. Going back to bugzilla discussion. From: Jiri Kosina jkos...@suse.cz Subject: [PATCH] drm/i915: read HEAD register back in init_ring_common() to enforce ordering Withtout this, ring initialization fails reliabily during resume with [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head ff8804 tail start 000e4000 Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..7add7ee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -517,6 +517,9 @@ static int init_ring_common(struct intel_engine_cs *ring) else ring_setup_phys_status_page(ring); + /* Enforce ordering by reading HEAD register back */ + I915_READ_HEAD(ring); + /* Initialize the ring. This must happen _after_ we've cleared the ring * registers with the above sequence (the readback of the HEAD registers * also enforces ordering), otherwise the hw might lose the new ring -- Jiri Kosina SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] drm/i915: Get CZ clock for VLV
CZ clock is related to data flow from memory to display plane. This is
required for comparison with CD clock before programming PFI credits.
v2: Ville's review comments
- Re-ordered CCK_CZ_CONTROL
- Refactored get_clock_speed
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 43 ++--
3 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dccd0a2..881e0a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2784,6 +2784,8 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv,
u32 reg, u32 val);
int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
+int valleyview_get_cz_clock_speed(struct drm_device *dev);
+
#define FORCEWAKE_RENDER (1 0)
#define FORCEWAKE_MEDIA(1 1)
#define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a8275b7..fb111cd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -616,6 +616,7 @@ enum punit_power_well {
#define DSI_PLL_N1_DIV_MASK (3 16)
#define DSI_PLL_M1_DIV_SHIFT 0
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
+#define CCK_CZ_CONTROL 0x62
#define CCK_DISPLAY_CLOCK_CONTROL 0x6b
#define CCK_TRUNK_FORCE_ON(1 17)
#define CCK_TRUNK_FORCE_OFF (1 16)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f1f1b54..2089319 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5316,30 +5316,59 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
return 0;
}
-static int valleyview_get_display_clock_speed(struct drm_device *dev)
+enum disp_clk {
+ CDCLK,
+ CZCLK
+};
+
+static int valleyview_get_cck_clock_speed(struct drm_device *dev,
+ enum disp_clk clk)
{
struct drm_i915_private *dev_priv = dev-dev_private;
int vco = valleyview_get_vco(dev_priv);
- u32 val;
+ u32 val, reg;
int divider;
- /* FIXME: Punit isn't quite ready yet */
- if (IS_CHERRYVIEW(dev))
- return 40;
+ switch(clk) {
+ case CDCLK:
+ default:
+ reg = CCK_DISPLAY_CLOCK_CONTROL;
+ break;
+ case CZCLK:
+ reg = CCK_CZ_CONTROL;
+ break;
+ }
+
mutex_lock(dev_priv-dpio_lock);
- val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+ val = vlv_cck_read(dev_priv, reg);
mutex_unlock(dev_priv-dpio_lock);
divider = val CCK_FREQUENCY_VALUES;
WARN((val CCK_FREQUENCY_STATUS) !=
(divider CCK_FREQUENCY_STATUS_SHIFT),
-cdclk change in progress\n);
+%sclk change in progress\n, (clk == CDCLK) ? cd : cz);
return DIV_ROUND_CLOSEST(vco 1, divider + 1);
}
+static int valleyview_get_display_clock_speed(struct drm_device *dev)
+{
+ /* FIXME: Punit isn't quite ready yet */
+ if (IS_CHERRYVIEW(dev))
+ return 40;
+ else
+ return valleyview_get_cck_clock_speed(dev, CDCLK);
+}
+
+int valleyview_get_cz_clock_speed(struct drm_device *dev)
+{
+ return valleyview_get_cck_clock_speed(dev, CZCLK);
+}
+
static int i945_get_display_clock_speed(struct drm_device *dev)
{
return 40;
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Renaming CCK related reg definitions
Rename the DISPLAY_TRUNK_* and DISPLAY_FREQUENCY_* bits to CCK_... instead
of DISPLAY_... to make it clear they apply to all CCK clock control registers.
Suggested by Ville.
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Ville Syrjä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 10 +-
drivers/gpu/drm/i915/intel_display.c | 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 187f862..a8275b7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -617,11 +617,11 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_SHIFT 0
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL 0x6b
-#define DISPLAY_TRUNK_FORCE_ON(1 17)
-#define DISPLAY_TRUNK_FORCE_OFF (1 16)
-#define DISPLAY_FREQUENCY_STATUS (0x1f 8)
-#define DISPLAY_FREQUENCY_STATUS_SHIFT8
-#define DISPLAY_FREQUENCY_VALUES (0x1f 0)
+#define CCK_TRUNK_FORCE_ON(1 17)
+#define CCK_TRUNK_FORCE_OFF (1 16)
+#define CCK_FREQUENCY_STATUS (0x1f 8)
+#define CCK_FREQUENCY_STATUS_SHIFT8
+#define CCK_FREQUENCY_VALUES (0x1f 0)
/**
* DOC: DPIO
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d828e40..f1f1b54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4503,12 +4503,12 @@ static void valleyview_set_cdclk(struct drm_device
*dev, int cdclk)
mutex_lock(dev_priv-dpio_lock);
/* adjust cdclk divider */
val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
- val = ~DISPLAY_FREQUENCY_VALUES;
+ val = ~CCK_FREQUENCY_VALUES;
val |= divider;
vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL, val);
if (wait_for((vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL)
- DISPLAY_FREQUENCY_STATUS) == (divider
DISPLAY_FREQUENCY_STATUS_SHIFT),
+ CCK_FREQUENCY_STATUS) == (divider
CCK_FREQUENCY_STATUS_SHIFT),
50))
DRM_ERROR(timed out waiting for CDclk change\n);
mutex_unlock(dev_priv-dpio_lock);
@@ -5331,10 +5331,10 @@ static int valleyview_get_display_clock_speed(struct
drm_device *dev)
val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
mutex_unlock(dev_priv-dpio_lock);
- divider = val DISPLAY_FREQUENCY_VALUES;
+ divider = val CCK_FREQUENCY_VALUES;
- WARN((val DISPLAY_FREQUENCY_STATUS) !=
-(divider DISPLAY_FREQUENCY_STATUS_SHIFT),
+ WARN((val CCK_FREQUENCY_STATUS) !=
+(divider CCK_FREQUENCY_STATUS_SHIFT),
cdclk change in progress\n);
return DIV_ROUND_CLOSEST(vco 1, divider + 1);
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Program PFI credits for VLV
From: Vidya Srinivas vidya.srini...@intel.com
PFI credit programming is required when CD clock (related to data flow from
display pipeline to end display) is greater than CZ clock (related to data
flow from memory to display plane). This programming should be done when all
planes are OFF to avoid intermittent hangs while accessing memory even from
different Gfx units (not just display).
If cdclk/czclk =1, PFI credits could be set as any number. To get better
performance, larger PFI credit can be assigned to PND. Otherwise if
cdclk/czclk1, the default PFI credit of 8 should be set.
v2:
- Change log to lower log level instead of DRM_ERROR
- Change function name to valleyview_program_pfi_credits
- Move program PFI credits to modeset_init instead of intel_set_mode
- Change magic numbers to logical constants
Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_display.c | 4 +++-
drivers/gpu/drm/i915/intel_pm.c | 22 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..00e0b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, false);
+
dev_priv-suspend_count++;
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, true);
+
intel_opregion_notify_adapter(dev, PCI_D0);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 881e0a6..88fd4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,6 +2172,8 @@ extern struct i915_params i915 __read_mostly;
/* i915_dma.c */
void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+ bool flag);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb111cd..7f4c2f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1937,6 +1937,11 @@ enum punit_power_well {
#define CZCLK_FREQ_MASK 0xf
#define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
+#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT (7 28)
+#define PFI_CREDIT_RESEND(1 27)
+#define VGA_FAST_MODE_DISABLE (1 14)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2089319..2af2e60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12691,8 +12691,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
intel_prepare_ddi(dev);
- if (IS_VALLEYVIEW(dev))
+ if (IS_VALLEYVIEW(dev)) {
vlv_update_cdclk(dev);
+ valleyview_program_pfi_credits(dev-dev_private, true);
+ }
intel_init_clock_gating(dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ba8dfeb..ad8e190 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7154,6 +7154,28 @@ void intel_fini_runtime_pm(struct drm_i915_private
*dev_priv)
pm_runtime_disable(device);
}
+void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+ bool flag)
+{
+ int cd_clk, cz_clk;
+
+ if (!flag) {
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+ return;
+ }
+
+ cd_clk = dev_priv-display.get_display_clock_speed(dev_priv-dev);
+ cz_clk = valleyview_get_cz_clock_speed(dev_priv-dev);
+
+ if (cd_clk = cz_clk) {
+ /* WA - write default credits before re-programming */
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE);
+ I915_WRITE(GCI_CONTROL, (PFI_CREDIT | PFI_CREDIT_RESEND |
+
[Intel-gfx] [PATCH] drm: Restore drm_file-is_master
Despite the claims of
commit 48ba813701eb14b3008edefef4a0789b328e278c
Author: David Herrmann dh.herrm...@gmail.com
Date: Tue Jul 22 18:46:09 2014 +0200
drm: drop redundant drm_file-is_master
drm_file-is_master is not synomous with having drm_file-master ==
drm_file-minor-master. This is because drm_file-master is the same
for all drm_files of the same generation and so when there is a master,
every drm_file believes itself to be the master. Confusion ensues and
things go pear shaped when one file is closed and there is no master
anymore.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82283
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Alex Deucher alexander.deuc...@amd.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: David Herrmann dh.herrm...@gmail.com
---
drivers/gpu/drm/drm_drv.c | 5 ++---
drivers/gpu/drm/drm_fops.c | 12 ++--
include/drm/drmP.h | 3 ++-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a1863d8..76cdb51 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -198,6 +198,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
if (unlikely(ret != 0))
drm_master_put(file_priv-minor-master);
}
+ file_priv-is_master = ret == 0;
out_unlock:
mutex_unlock(dev-master_mutex);
@@ -213,13 +214,11 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void
*data,
if (!drm_is_master(file_priv))
goto out_unlock;
- if (!file_priv-minor-master)
- goto out_unlock;
-
ret = 0;
if (dev-driver-master_drop)
dev-driver-master_drop(dev, file_priv, false);
drm_master_put(file_priv-minor-master);
+ file_priv-is_master = false;
out_unlock:
mutex_unlock(dev-master_mutex);
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index c0166ba..083f7b9 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -196,6 +196,7 @@ static int drm_open_helper(struct file *filp, struct
drm_minor *minor)
/* take another reference for the copy in the local file priv */
priv-master = drm_master_get(priv-minor-master);
+ priv-is_master = true;
priv-authenticated = 1;
if (dev-driver-master_create) {
@@ -434,12 +435,11 @@ int drm_release(struct inode *inode, struct file *filp)
}
mutex_unlock(dev-struct_mutex);
- if (file_priv-minor-master == file_priv-master) {
- /* drop the reference held my the minor */
- if (dev-driver-master_drop)
- dev-driver-master_drop(dev, file_priv, true);
- drm_master_put(file_priv-minor-master);
- }
+ /* drop the reference held my the minor */
+ if (dev-driver-master_drop)
+ dev-driver-master_drop(dev, file_priv, true);
+ drm_master_put(file_priv-minor-master);
+ file_priv-is_master = false;
}
/* drop the master reference held by the file priv */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0ffce5c..70a6598 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -378,6 +378,7 @@ struct drm_prime_file_private {
/** File private data */
struct drm_file {
+ bool is_master:1;
unsigned authenticated :1;
/* true when the client has asked us to expose stereo 3D mode flags */
unsigned stereo_allowed :1;
@@ -1177,7 +1178,7 @@ static inline bool drm_is_primary_client(const struct
drm_file *file_priv)
*/
static inline bool drm_is_master(const struct drm_file *file)
{
- return file-master file-master == file-minor-master;
+ return file-is_master;
}
/**/
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] igt_kms: don't get drmModeRes just to free it later
From: Paulo Zanoni paulo.r.zan...@intel.com
Stress testing malloc is not our goal :)
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
lib/igt_kms.c | 12
1 file changed, 12 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 740b5dd..8ab729b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -276,21 +276,11 @@ void igt_set_vt_graphics_mode(void)
int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
drmModeModeInfo *mode)
{
- drmModeRes *resources;
int i;
- resources = drmModeGetResources(drm_fd);
- if (!resources) {
- perror(drmModeGetResources failed);
-
- return -1;
- }
-
if (!connector-count_modes) {
fprintf(stderr, no modes for connector %d\n,
connector-connector_id);
- drmModeFreeResources(resources);
-
return -1;
}
@@ -303,8 +293,6 @@ int kmstest_get_connector_default_mode(int drm_fd,
drmModeConnector *connector,
}
}
- drmModeFreeResources(resources);
-
return 0;
}
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] igt_kms: optionally return the property from get_property
From: Paulo Zanoni paulo.r.zan...@intel.com
So we can use this function on places that also need the property
pointer, without having to call drmModeGetProperty() again with the
returned id.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
lib/igt_kms.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5dd67fe..664b9e8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -595,42 +595,46 @@ static void igt_output_refresh(igt_output_t *output)
static bool
get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
const char *name, uint32_t *prop_id /* out */,
-uint64_t *value /* out */)
+uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
{
drmModeObjectPropertiesPtr proplist;
- drmModePropertyPtr prop = NULL;
+ drmModePropertyPtr _prop;
bool found = false;
int i;
proplist = drmModeObjectGetProperties(drm_fd, object_id, object_type);
for (i = 0; i proplist-count_props; i++) {
- drmModeFreeProperty(prop);
- prop = drmModeGetProperty(drm_fd, proplist-props[i]);
- if (!prop)
+ _prop = drmModeGetProperty(drm_fd, proplist-props[i]);
+ if (!_prop)
continue;
- if (strcmp(prop-name, name) == 0) {
+ if (strcmp(_prop-name, name) == 0) {
found = true;
if (prop_id)
*prop_id = proplist-props[i];
if (value)
*value = proplist-prop_values[i];
- goto out;
+ if (prop)
+ *prop = _prop;
+ else
+ drmModeFreeProperty(_prop);
+
+ break;
}
+ drmModeFreeProperty(_prop);
}
-out:
- drmModeFreeProperty(prop);
drmModeFreeObjectProperties(proplist);
return found;
}
static bool
get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
- uint32_t *prop_id /* out */, uint64_t *value /* out */)
+ uint32_t *prop_id /* out */, uint64_t *value /* out */,
+ drmModePropertyPtr *prop /* out */)
{
return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
- name, prop_id, value);
+ name, prop_id, value, prop);
}
static void
@@ -654,7 +658,7 @@ static int get_drm_plane_type(int drm_fd, uint32_t plane_id)
bool has_prop;
has_prop = get_plane_property(drm_fd, plane_id, type,
- NULL /* prop_id */, value);
+ NULL /* prop_id */, value, NULL);
if (has_prop)
return (int)value;
@@ -743,7 +747,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
get_plane_property(display-drm_fd, drm_plane-plane_id,
rotation,
plane-rotation_property,
- prop_value);
+ prop_value,
+ NULL);
plane-rotation = (igt_rotation_t)prop_value;
}
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] igt_kms: pass drm_fd instead of igt_display_t on some functions
From: Paulo Zanoni paulo.r.zan...@intel.com
Since these functions only really use the drm_fd. The goal is to be
able to reuse these functions on programs that don't use the
igt_display_t structure.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
lib/igt_kms.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8ab729b..5dd67fe 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -593,20 +593,19 @@ static void igt_output_refresh(igt_output_t *output)
}
static bool
-get_property(igt_display_t *display,
-uint32_t object_id, uint32_t object_type, const char *name,
-uint32_t *prop_id /* out */, uint64_t *value /* out */)
+get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+const char *name, uint32_t *prop_id /* out */,
+uint64_t *value /* out */)
{
drmModeObjectPropertiesPtr proplist;
drmModePropertyPtr prop = NULL;
bool found = false;
int i;
- proplist = drmModeObjectGetProperties(display-drm_fd,
- object_id, object_type);
+ proplist = drmModeObjectGetProperties(drm_fd, object_id, object_type);
for (i = 0; i proplist-count_props; i++) {
drmModeFreeProperty(prop);
- prop = drmModeGetProperty(display-drm_fd, proplist-props[i]);
+ prop = drmModeGetProperty(drm_fd, proplist-props[i]);
if (!prop)
continue;
@@ -627,10 +626,10 @@ out:
}
static bool
-get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
+get_plane_property(int drm_fd, uint32_t plane_id, const char *name,
uint32_t *prop_id /* out */, uint64_t *value /* out */)
{
- return get_property(display, plane_id, DRM_MODE_OBJECT_PLANE,
+ return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
name, prop_id, value);
}
@@ -649,12 +648,12 @@ igt_plane_set_property(igt_plane_t *plane, uint32_t
prop_id, uint64_t value)
* find a type property, then the kernel doesn't support universal
* planes and we know the plane is an overlay/sprite.
*/
-static int get_drm_plane_type(igt_display_t *display, uint32_t plane_id)
+static int get_drm_plane_type(int drm_fd, uint32_t plane_id)
{
uint64_t value;
bool has_prop;
- has_prop = get_plane_property(display, plane_id, type,
+ has_prop = get_plane_property(drm_fd, plane_id, type,
NULL /* prop_id */, value);
if (has_prop)
return (int)value;
@@ -710,7 +709,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
continue;
}
- type = get_drm_plane_type(display,
+ type = get_drm_plane_type(display-drm_fd,
plane_resources-planes[j]);
switch (type) {
case DRM_PLANE_TYPE_PRIMARY:
@@ -741,7 +740,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
plane-pipe = pipe;
plane-drm_plane = drm_plane;
- get_plane_property(display, drm_plane-plane_id,
+ get_plane_property(display-drm_fd, drm_plane-plane_id,
rotation,
plane-rotation_property,
prop_value);
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] igt_kms: document and export igt_get_property()
From: Paulo Zanoni paulo.r.zan...@intel.com
So we can use it on pm_rpm.c.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
lib/igt_kms.c | 27 +--
lib/igt_kms.h | 4
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 664b9e8..6cca7e8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -592,10 +592,25 @@ static void igt_output_refresh(igt_output_t *output)
display-pipes_in_use |= 1 output-config.pipe;
}
-static bool
-get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
-const char *name, uint32_t *prop_id /* out */,
-uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
+/**
+ * igt_get_property:
+ * @drm_fd: drm file descriptor
+ * @object_id: object whose properties we're going to get
+ * @object_type: type of obj_id (DRM_MODE_OBJECT_*)
+ * @name: name of the property we're going to get
+ * @prop_id: if not NULL, returns the property id
+ * @value: if not NULL, returns the property value
+ * @prop: if not NULL, returns the property, and the caller will have to free
+ *it manually.
+ *
+ * Finds a property with the given name on the given object.
+ *
+ * Returns: true in case we found something.
+ */
+bool
+igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+const char *name, uint32_t *prop_id /* out */,
+uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
{
drmModeObjectPropertiesPtr proplist;
drmModePropertyPtr _prop;
@@ -633,8 +648,8 @@ get_plane_property(int drm_fd, uint32_t plane_id, const
char *name,
uint32_t *prop_id /* out */, uint64_t *value /* out */,
drmModePropertyPtr *prop /* out */)
{
- return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
- name, prop_id, value, prop);
+ return igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+ name, prop_id, value, prop);
}
static void
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 08b46ab..f8c500e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -255,5 +255,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
void igt_enable_connectors(void);
void igt_reset_connectors(void);
+bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+ const char *name, uint32_t *prop_id, uint64_t *value,
+ drmModePropertyPtr *prop);
+
#endif /* __IGT_KMS_H__ */
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] igt_kms: add igt_unset_all_crtcs()
From: Paulo Zanoni paulo.r.zan...@intel.com
Both pm_rpm.c and pm_lpsp.c call it disable_all_screens, but let's
give it a name that better describes what the implementation does.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
lib/igt_kms.c | 18 ++
lib/igt_kms.h | 1 +
tests/pm_lpsp.c | 17 +++--
tests/pm_rpm.c | 8 +---
4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6cca7e8..678b3cd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1560,3 +1560,21 @@ void igt_reset_connectors(void)
close(drm_fd);
}
+
+/**
+ * igt_unset_all_crtcs:
+ * @drm_fd: the DRM fd
+ * @resources: libdrm resources pointer
+ *
+ * Disables all the screens.
+ */
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources)
+{
+ int i, rc;
+
+ for (i = 0; i resources-count_crtcs; i++) {
+ rc = drmModeSetCrtc(drm_fd, resources-crtcs[i], -1, 0, 0, NULL,
+ 0, NULL);
+ igt_assert(rc == 0);
+ }
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f8c500e..43e84f6 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -258,6 +258,7 @@ void igt_reset_connectors(void);
bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
const char *name, uint32_t *prop_id, uint64_t *value,
drmModePropertyPtr *prop);
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources);
#endif /* __IGT_KMS_H__ */
diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
index 9d3884c..b3a21ea 100644
--- a/tests/pm_lpsp.c
+++ b/tests/pm_lpsp.c
@@ -70,22 +70,11 @@ static bool lpsp_is_enabled(int drm_fd)
return !(val HSW_PWR_WELL_STATE_ENABLED);
}
-static void disable_all_screens(int drm_fd, drmModeResPtr drm_resources)
-{
- int i, rc;
-
- for (i = 0; i drm_resources-count_crtcs; i++) {
- rc = drmModeSetCrtc(drm_fd, drm_resources-crtcs[i], -1, 0, 0,
- NULL, 0, NULL);
- igt_assert(rc == 0);
- }
-}
-
/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
* low power mode when no pipes are enabled, so do this check anyway. */
static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
{
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
igt_assert(lpsp_is_enabled(drm_fd));
}
@@ -131,7 +120,7 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
.name = Custom 1024x768,
};
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
for (i = 0; i drm_res-count_connectors; i++) {
drmModeConnectorPtr c = drm_connectors[i];
@@ -193,7 +182,7 @@ static void non_edp_subtest(int drm_fd, drmModeResPtr
drm_res,
uint32_t connector_id = 0, crtc_id = 0, buffer_id = 0;
drmModeModeInfoPtr mode = NULL;
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
for (i = 0; i drm_res-count_connectors; i++) {
drmModeConnectorPtr c = drm_connectors[i];
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 800ab4b..84c71bd 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -232,13 +232,7 @@ static void disable_all_screens_dpms(struct mode_set_data
*data)
static void disable_all_screens(struct mode_set_data *data)
{
- int i, rc;
-
- for (i = 0; i data-res-count_crtcs; i++) {
- rc = drmModeSetCrtc(drm_fd, data-res-crtcs[i], -1, 0, 0,
- NULL, 0, NULL);
- igt_assert(rc == 0);
- }
+ igt_unset_all_crtcs(drm_fd, data-res);
}
static struct scanout_fb *create_fb(struct mode_set_data *data, int width,
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] tests/pm_rpm: use igt_get_property()
From: Paulo Zanoni paulo.r.zan...@intel.com
So we can reduce the code size.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
tests/pm_rpm.c | 97 --
1 file changed, 40 insertions(+), 57 deletions(-)
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 0633476..800ab4b 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -348,29 +348,25 @@ static void enable_one_screen(struct mode_set_data *data)
static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
int index)
{
- unsigned int i;
- drmModeObjectPropertiesPtr props;
- drmModePropertyBlobPtr ret = NULL;
+ bool found;
+ uint64_t prop_value;
+ drmModePropertyPtr prop;
+ drmModePropertyBlobPtr blob = NULL;
- props = drmModeObjectGetProperties(drm_fd, connector-connector_id,
- DRM_MODE_OBJECT_CONNECTOR);
+ found = igt_get_property(drm_fd, connector-connector_id,
+DRM_MODE_OBJECT_CONNECTOR, EDID,
+NULL, prop_value, prop);
- for (i = 0; i props-count_props; i++) {
- drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
-props-props[i]);
+ if (found) {
+ igt_assert(prop-flags DRM_MODE_PROP_BLOB);
+ igt_assert(prop-count_blobs == 0);
- if (strcmp(prop-name, EDID) == 0) {
- igt_assert(prop-flags DRM_MODE_PROP_BLOB);
- igt_assert(prop-count_blobs == 0);
- ret = drmModeGetPropertyBlob(drm_fd,
-props-prop_values[i]);
- }
+ blob = drmModeGetPropertyBlob(drm_fd, prop_value);
drmModeFreeProperty(prop);
}
- drmModeFreeObjectProperties(props);
- return ret;
+ return blob;
}
static void init_mode_set_data(struct mode_set_data *data)
@@ -1529,52 +1525,39 @@ static void cursor_subtest(bool dpms)
static enum plane_type get_plane_type(uint32_t plane_id)
{
- drmModeObjectPropertiesPtr props;
- int i, j;
+ int i;
+ bool found;
+ uint64_t prop_value;
+ drmModePropertyPtr prop;
+ const char *enum_name = NULL;
enum plane_type type;
- bool found = false;
-
- props = drmModeObjectGetProperties(drm_fd, plane_id,
- DRM_MODE_OBJECT_PLANE);
- igt_assert(props);
-
- for (i = 0; i props-count_props !found; i++) {
- drmModePropertyPtr prop;
- const char *enum_name = NULL;
-
- prop = drmModeGetProperty(drm_fd, props-props[i]);
- igt_assert(prop);
-
- if (strcmp(prop-name, type) == 0) {
- igt_assert(prop-flags DRM_MODE_PROP_ENUM);
- igt_assert(props-prop_values[i] prop-count_enums);
-
- for (j = 0; j prop-count_enums; j++) {
- if (prop-enums[j].value ==
- props-prop_values[i]) {
- enum_name = prop-enums[j].name;
- break;
- }
- }
- igt_assert(enum_name);
-
- if (strcmp(enum_name, Overlay) == 0)
- type = PLANE_OVERLAY;
- else if (strcmp(enum_name, Primary) == 0)
- type = PLANE_PRIMARY;
- else if (strcmp(enum_name, Cursor) == 0)
- type = PLANE_CURSOR;
- else
- igt_assert(0);
- found = true;
- }
+ found = igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+type, NULL, prop_value, prop);
+ igt_assert(found);
- drmModeFreeProperty(prop);
+ igt_assert(prop-flags DRM_MODE_PROP_ENUM);
+ igt_assert(prop_value prop-count_enums);
+
+ for (i = 0; i prop-count_enums; i++) {
+ if (prop-enums[i].value == prop_value) {
+ enum_name = prop-enums[i].name;
+ break;
+ }
}
- igt_assert(found);
+ igt_assert(enum_name);
+
+ if (strcmp(enum_name, Overlay) == 0)
+ type = PLANE_OVERLAY;
+ else if (strcmp(enum_name, Primary) == 0)
+ type = PLANE_PRIMARY;
+ else if (strcmp(enum_name, Cursor) == 0)
+ type = PLANE_CURSOR;
+ else
+ igt_assert(0);
+
+ drmModeFreeProperty(prop);
- drmModeFreeObjectProperties(props);
return
[Intel-gfx] [PATCH] tests/pm_rpm: add subtests for fences
From: Paulo Zanoni paulo.r.zan...@intel.com
Daniel thought fences were not surviving runtime PM on DPMS, so we
wrote this test to check. The good news is that the test passed, so
there's no need for a Kernel patch, at least on the Kernel I tested.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
tests/pm_rpm.c | 72 ++
1 file changed, 72 insertions(+)
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 84c71bd..5e520da 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -1704,6 +1704,74 @@ static void planes_subtest(bool universal, bool dpms)
}
}
+static void fences_subtest(bool dpms)
+{
+ uint32_t connector_id, crtc_id = 0;
+ drmModeModeInfoPtr mode;
+ struct igt_fb scanout_fb;
+ int rc, i;
+ uint32_t *buf_ptr;
+ uint32_t tiling = false, swizzle;
+
+ disable_all_screens(ms_data);
+ igt_assert(wait_for_suspended());
+
+ igt_require(find_connector_for_modeset(ms_data, SCREEN_TYPE_ANY,
+ connector_id, mode));
+
+ crtc_id = ms_data.res-crtcs[0];
+ igt_assert(crtc_id);
+
+ igt_create_fb(drm_fd, mode-hdisplay, mode-vdisplay,
+ DRM_FORMAT_XRGB, true, scanout_fb);
+
+ /* Even though we passed true as the tiling argument, double-check
+* that the fb is really tiled. */
+ gem_get_tiling(drm_fd, scanout_fb.gem_handle, tiling, swizzle);
+ igt_assert(tiling);
+
+ buf_ptr = gem_mmap__gtt(drm_fd, scanout_fb.gem_handle,
+ scanout_fb.size, PROT_WRITE | PROT_READ);
+ for (i = 0; i scanout_fb.size/sizeof(uint32_t); i++)
+ buf_ptr[i] = i;
+
+ rc = drmModeSetCrtc(drm_fd, crtc_id, scanout_fb.fb_id, 0, 0,
+ connector_id, 1, mode);
+ igt_assert(rc == 0);
+ igt_assert(wait_for_active());
+
+ if (dpms)
+ disable_all_screens_dpms(ms_data);
+ else
+ disable_all_screens(ms_data);
+ igt_assert(wait_for_suspended());
+
+ for (i = 0; i scanout_fb.size/sizeof(uint32_t); i++)
+ igt_assert_eq(buf_ptr[i], i);
+ igt_assert(wait_for_suspended());
+
+ if (dpms) {
+ drmModeConnectorPtr c = NULL;
+
+ for (i = 0; i ms_data.res-count_connectors; i++)
+ if (ms_data.connectors[i]-connector_id == connector_id)
+ c = ms_data.connectors[i];
+ igt_assert(c);
+
+ kmstest_set_connector_dpms(drm_fd, c, DRM_MODE_DPMS_ON);
+ } else {
+ rc = drmModeSetCrtc(drm_fd, crtc_id, scanout_fb.fb_id, 0, 0,
+ connector_id, 1, mode);
+ igt_assert(rc == 0);
+ }
+ igt_assert(wait_for_active());
+
+ for (i = 0; i scanout_fb.size/sizeof(uint32_t); i++)
+ igt_assert_eq(buf_ptr[i], i);
+
+ igt_assert(munmap(buf_ptr, scanout_fb.size) == 0);
+}
+
int rounds = 50;
bool stay = false;
@@ -1808,6 +1876,10 @@ int main(int argc, char *argv[])
dpms_mode_unset_subtest(SCREEN_TYPE_LPSP);
igt_subtest(dpms-mode-unset-non-lpsp)
dpms_mode_unset_subtest(SCREEN_TYPE_NON_LPSP);
+ igt_subtest(fences)
+ fences_subtest(false);
+ igt_subtest(fences-dpms)
+ fences_subtest(true);
/* Modeset stress */
igt_subtest(modeset-lpsp-stress)
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Prevent recursive deadlock on releasing a busy userptr
During release of the GEM object we hold the struct_mutex. As the
object may be holding onto the last reference for the task-mm,
calling mmput() may trigger exit_mmap() which close the vma
which will call drm_gem_vm_close() and attempt to reacquire
the struct_mutex. In order to avoid that recursion, we have
to defer the mmput() until after we drop the struct_mutex,
i.e. we need to schedule a worker to do the clean up. A further issue
spotted by Tvrtko was caused when we took a GTT mmapping of a userptr
buffer object. In that case, we would never call mmput as the object
would be cyclically referenced by the GTT mmapping and not freed upon
process exit - keeping the entire process mm alive after the process
task was reaped. The fix employed is to replace the mm_users/mmput()
reference handling to mm_count/mmdrop() for the shared i915_mm_struct.
INFO: task test_surfaces:1632 blocked for more than 120 seconds.
Tainted: GF O 3.14.5+ #1
echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message.
test_surfaces D 0 1632 1590 0x0082
88014914baa8 0046 88014914a010
00012c40 00012c40 8800a0058210 88014784b010
88014914a010 880037b1c820 8800a0058210 880037b1c824
Call Trace:
[81582499] schedule+0x29/0x70
[815825fe] schedule_preempt_disabled+0xe/0x10
[81583b93] __mutex_lock_slowpath+0x183/0x220
[81583c53] mutex_lock+0x23/0x40
[a005c2a3] drm_gem_vm_close+0x33/0x70 [drm]
[8115a483] remove_vma+0x33/0x70
[8115a5dc] exit_mmap+0x11c/0x170
[8104d6eb] mmput+0x6b/0x100
[a00f44b9] i915_gem_userptr_release+0x89/0xc0 [i915]
[a00e6706] i915_gem_free_object+0x126/0x250 [i915]
[a005c06a] drm_gem_object_free+0x2a/0x40 [drm]
[a005cc32] drm_gem_object_handle_unreference_unlocked+0xe2/0x120
[drm]
[a005ccd4] drm_gem_object_release_handle+0x64/0x90 [drm]
[8127ffeb] idr_for_each+0xab/0x100
[a005cc70] ?
drm_gem_object_handle_unreference_unlocked+0x120/0x120 [drm]
[81583c46] ? mutex_lock+0x16/0x40
[a005c354] drm_gem_release+0x24/0x40 [drm]
[a005b82b] drm_release+0x3fb/0x480 [drm]
[8118d482] __fput+0xb2/0x260
[8118d6de] fput+0xe/0x10
[8106f27f] task_work_run+0x8f/0xf0
[81052228] do_exit+0x1a8/0x480
[81052551] do_group_exit+0x51/0xc0
[810525d7] SyS_exit_group+0x17/0x20
[8158e092] system_call_fastpath+0x16/0x1b
v2: Incorporate feedback from Tvrtko and remove the unnessary mm
referencing when creating the i915_mm_struct and improve some of the
function names and comments.
Reported-by: Jacek Danecki jacek.dane...@intel.com
Test-case: igt/gem_userptr_blits/process-exit*
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Tested-by: Gong, Zhipeng zhipeng.g...@intel.com
Cc: Jacek Danecki jacek.dane...@intel.com
Cc: Ursulin, Tvrtko tvrtko.ursu...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 409 ++--
2 files changed, 235 insertions(+), 184 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1d29f5..299233e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -190,6 +190,7 @@ enum hpd_pin {
if ((1 (domain)) (mask))
struct drm_i915_private;
+struct i915_mm_struct;
struct i915_mmu_object;
enum intel_dpll_id {
@@ -1536,9 +1537,8 @@ struct drm_i915_private {
struct i915_gtt gtt; /* VM representing the global address space */
struct i915_gem_mm mm;
-#if defined(CONFIG_MMU_NOTIFIER)
- DECLARE_HASHTABLE(mmu_notifiers, 7);
-#endif
+ DECLARE_HASHTABLE(mm_structs, 7);
+ struct mutex mm_lock;
/* Kernel Modesetting */
@@ -1845,8 +1845,8 @@ struct drm_i915_gem_object {
unsigned workers :4;
#define I915_GEM_USERPTR_MAX_WORKERS 15
- struct mm_struct *mm;
- struct i915_mmu_object *mn;
+ struct i915_mm_struct *mm;
+ struct i915_mmu_object *mmu_object;
struct work_struct *work;
} userptr;
};
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index fe69fc8..d384139 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -32,6 +32,15 @@
#include linux/mempolicy.h
#include linux/swap.h
+struct i915_mm_struct {
+ struct mm_struct *mm;
+ struct drm_device *dev;
+ struct i915_mmu_notifier *mn;
+ struct hlist_node node;
+ struct kref kref;
+ struct work_struct work;
+};
+
#if
[Intel-gfx] [PATCH] drm/i915: Idleness detection for DRRS
Adding support to detect display idleness by tracking page flip from
user space. Switch to low refresh rate is triggered after 2 seconds of
idleness. The delay is configurable. If there is a page flip or call to
update the plane, then high refresh rate is applied.
The feature is not used in dual-display mode.
v2: Chris Wilson's review comments incorporated.
Modify idleness detection implementation to make it similar to the
implementation of intel_update_fbc/intel_disable_fbc
v3: Internal review comments incorporated
Add NULL pointer check in intel_disable_drrs.
Add drrs calls in i9xx_crtc_enable/disable and valleyview_crtc_enable.
v4: Jani's review comments incorporated.
Change in sequence in intel_update_drrs. Comment modified to remove details
of update param. Modified DRRS idleness interval to a module parameter.
v5: Chris's review comments incorporated.
Initialize connector in idleness detection init. Modifications made to
use only intel_connector in i915_drrs and derive intel_dp when required.
Added a function drrs_fini to cleanup DRRS work.
v6: Internal review comments. Removed check for primary enabled, which is
a redundant check, in the case of clone mode. Added a flag to track
dual-display configuration. Remove print statement for cancel DRR work
and print DRRS not supported only once.
v7: As per internal review comments, removing calls to update/disable drrs
from sprite update path. For sprite, all drrs related updates would be
taken care of with calls to crtc page flip itself. This will have to be
revisited later if flip infrastructure changes for sprite.
v8: Incorporated Jani's review comments. Added space after the periods in the
module param description. Changes around drrs-fini to remove seamless DRRS
check.
v9: Added checks for PSR before updating DRRS. Added check for module
param drrs_interval before updating DRRS (this is required if the interval
is modified by the user during system use). DRRS disabled by default. Changes
based on Jani's review comments
v10: Disable/enable DRRS when PSR is enable/disabled.
v11: Moved DRRS not supported log to patch2.
Patch rebased.
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Reviewed-by: Jani Nikula jani.nik...@intel.com
Cc: Daniel Vetter dan...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++
drivers/gpu/drm/i915/i915_params.c | 8 ++
drivers/gpu/drm/i915/intel_display.c | 13
drivers/gpu/drm/i915/intel_dp.c | 27 ++-
drivers/gpu/drm/i915/intel_drv.h | 5 +-
drivers/gpu/drm/i915/intel_pm.c | 142 +++
6 files changed, 200 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 125a83c..993fdb1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -667,6 +667,12 @@ struct i915_fbc {
struct i915_drrs {
struct intel_connector *connector;
+ bool is_clone;
+ struct intel_drrs_work {
+ struct delayed_work work;
+ struct drm_crtc *crtc;
+ int interval;
+ } *drrs_work;
};
struct intel_dp;
@@ -2156,6 +2162,7 @@ struct i915_params {
int enable_ips;
int invert_brightness;
int enable_cmd_parser;
+ int drrs_interval;
/* leave bools at the end to not create holes */
bool enable_hangcheck;
bool fastboot;
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 62ee830..912a02f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -50,6 +50,7 @@ struct i915_params i915 __read_mostly = {
.disable_vtd_wa = 0,
.use_mmio_flip = 0,
.mmio_debug = 0,
+ .drrs_interval = 0,
};
module_param_named(modeset, i915.modeset, int, 0400);
@@ -167,3 +168,10 @@ module_param_named(mmio_debug, i915.mmio_debug, bool,
0600);
MODULE_PARM_DESC(mmio_debug,
Enable the MMIO debug code (default: false). This may negatively
affect performance.);
+
+module_param_named(drrs_interval, i915.drrs_interval, int, 0600);
+MODULE_PARM_DESC(drrs_interval,
+ DRRS idleness detection interval (default: 0 ms).
+ If this field is set to 0, then seamless DRRS feature
+ based on idleness detection is disabled.
+ The interval is to be set in milliseconds.);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..da24f4e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2743,6 +2743,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
mutex_lock(dev-struct_mutex);
intel_update_fbc(dev);
+ intel_update_drrs(dev);
mutex_unlock(dev-struct_mutex);
return 0;
@@ -3897,6 +3898,7 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
Re: [Intel-gfx] [PATCH] drm/i915: Idleness detection for DRRS
On Thu, Aug 07, 2014 at 07:15:15PM +0530, Vandana Kannan wrote: Adding support to detect display idleness by tracking page flip from user space. Switch to low refresh rate is triggered after 2 seconds of idleness. The delay is configurable. If there is a page flip or call to update the plane, then high refresh rate is applied. The feature is not used in dual-display mode. v2: Chris Wilson's review comments incorporated. Modify idleness detection implementation to make it similar to the implementation of intel_update_fbc/intel_disable_fbc v3: Internal review comments incorporated Add NULL pointer check in intel_disable_drrs. Add drrs calls in i9xx_crtc_enable/disable and valleyview_crtc_enable. v4: Jani's review comments incorporated. Change in sequence in intel_update_drrs. Comment modified to remove details of update param. Modified DRRS idleness interval to a module parameter. v5: Chris's review comments incorporated. Initialize connector in idleness detection init. Modifications made to use only intel_connector in i915_drrs and derive intel_dp when required. Added a function drrs_fini to cleanup DRRS work. v6: Internal review comments. Removed check for primary enabled, which is a redundant check, in the case of clone mode. Added a flag to track dual-display configuration. Remove print statement for cancel DRR work and print DRRS not supported only once. v7: As per internal review comments, removing calls to update/disable drrs from sprite update path. For sprite, all drrs related updates would be taken care of with calls to crtc page flip itself. This will have to be revisited later if flip infrastructure changes for sprite. v8: Incorporated Jani's review comments. Added space after the periods in the module param description. Changes around drrs-fini to remove seamless DRRS check. v9: Added checks for PSR before updating DRRS. Added check for module param drrs_interval before updating DRRS (this is required if the interval is modified by the user during system use). DRRS disabled by default. Changes based on Jani's review comments v10: Disable/enable DRRS when PSR is enable/disabled. v11: Moved DRRS not supported log to patch2. Patch rebased. Signed-off-by: Vandana Kannan vandana.kan...@intel.com Signed-off-by: Pradeep Bhat pradeep.b...@intel.com Reviewed-by: Jani Nikula jani.nik...@intel.com Cc: Daniel Vetter dan...@ffwll.ch Woohoo, we still want to introduce yet another idleness detector and ignore the complexity of front buffer rendering. -Chrsi -- 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: Idleness detection for DRRS
Hi Vandana,
I think we need to start over from a freshclean slate with this part
- too much changed and all the differen revisions of this patch don't
provide that much information.
Also as Chris says this introduces yet another idleness detection
system, and we already have plenty of them.
High-level comments:
- We now have the frontbuffer tracking infrastructure, see the various
new intel_frontbuffer_* and similar functions in intel_display.c. They
have nice kerneldoc explaining what exactly they do. We need to hook
into that instead of creating new idlesness detection.
- There's already calls to intel_increase/decrease_pllclock. That
should be renamed to gmch_increase/decrease_pllclock, and we the new
dp drrs support here should be called at _exactly_ the same spots.
- This doesn't use the pipe config we've added at all - DRRS shouldn't
look at the panel, but only at the pipe config to figure out whether
drrs is supported on a given pipe or not. There is also no need to
restrict DRRS to single pipe mode at all.
- Locking is important. In case of doubt follow the example
established by the new PSR code. Especially important is to not
require modeset locks.
- There's no need to disable DRRS from the PSR (or fbc) code at all,
the frontbuffer tracking code will take care of all this. And just
using pageflips as business signal isn't good enough.
This is just a rough guideline, I think it'd be good to first hash out
the rough design in more detail (maybe as a patch with just
pseudo-code) before starting with codeing.
Thanks, Daniel
On Thu, Aug 7, 2014 at 3:45 PM, Vandana Kannan vandana.kan...@intel.com wrote:
Adding support to detect display idleness by tracking page flip from
user space. Switch to low refresh rate is triggered after 2 seconds of
idleness. The delay is configurable. If there is a page flip or call to
update the plane, then high refresh rate is applied.
The feature is not used in dual-display mode.
v2: Chris Wilson's review comments incorporated.
Modify idleness detection implementation to make it similar to the
implementation of intel_update_fbc/intel_disable_fbc
v3: Internal review comments incorporated
Add NULL pointer check in intel_disable_drrs.
Add drrs calls in i9xx_crtc_enable/disable and valleyview_crtc_enable.
v4: Jani's review comments incorporated.
Change in sequence in intel_update_drrs. Comment modified to remove details
of update param. Modified DRRS idleness interval to a module parameter.
v5: Chris's review comments incorporated.
Initialize connector in idleness detection init. Modifications made to
use only intel_connector in i915_drrs and derive intel_dp when required.
Added a function drrs_fini to cleanup DRRS work.
v6: Internal review comments. Removed check for primary enabled, which is
a redundant check, in the case of clone mode. Added a flag to track
dual-display configuration. Remove print statement for cancel DRR work
and print DRRS not supported only once.
v7: As per internal review comments, removing calls to update/disable drrs
from sprite update path. For sprite, all drrs related updates would be
taken care of with calls to crtc page flip itself. This will have to be
revisited later if flip infrastructure changes for sprite.
v8: Incorporated Jani's review comments. Added space after the periods in the
module param description. Changes around drrs-fini to remove seamless DRRS
check.
v9: Added checks for PSR before updating DRRS. Added check for module
param drrs_interval before updating DRRS (this is required if the interval
is modified by the user during system use). DRRS disabled by default. Changes
based on Jani's review comments
v10: Disable/enable DRRS when PSR is enable/disabled.
v11: Moved DRRS not supported log to patch2.
Patch rebased.
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Reviewed-by: Jani Nikula jani.nik...@intel.com
Cc: Daniel Vetter dan...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 7 ++
drivers/gpu/drm/i915/i915_params.c | 8 ++
drivers/gpu/drm/i915/intel_display.c | 13
drivers/gpu/drm/i915/intel_dp.c | 27 ++-
drivers/gpu/drm/i915/intel_drv.h | 5 +-
drivers/gpu/drm/i915/intel_pm.c | 142
+++
6 files changed, 200 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 125a83c..993fdb1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -667,6 +667,12 @@ struct i915_fbc {
struct i915_drrs {
struct intel_connector *connector;
+ bool is_clone;
+ struct intel_drrs_work {
+ struct delayed_work work;
+ struct drm_crtc *crtc;
+ int interval;
+ } *drrs_work;
};
struct intel_dp;
@@ -2156,6 +2162,7 @@ struct i915_params {
int enable_ips;
int
[Intel-gfx] [PATCH] drm/i915: No busy-loop wait_for in the ring init code
Doing a 1s wait (tops) with the cpu is a bit excessive. Tune it down
like everything else in that code.
Cc: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05969f03c0c1..966d8f72da45 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -476,7 +476,7 @@ static bool stop_ring(struct intel_engine_cs *ring)
if (!IS_GEN2(ring-dev)) {
I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
- if (wait_for_atomic((I915_READ_MODE(ring) MODE_IDLE) != 0,
1000)) {
+ if (wait_for((I915_READ_MODE(ring) MODE_IDLE) != 0, 1000)) {
DRM_ERROR(%s :timed out trying to stop ring\n,
ring-name);
return false;
}
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START
Ville found an old w/a documented for g4x that suggested that we need to
reset the HEAD after writing START. This is a useful fixup for some of
the g4x ring initialisation woes, but as usual, not all.
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 45e3ec927051..0a37ac0d9c0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring)
* also enforces ordering), otherwise the hw might lose the new ring
* register values. */
I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj));
+
+ /* WaClearRingBufHeadRegAtInit:ctg,elk */
+ if (I915_READ_HEAD(ring)) {
+ DRM_DEBUG(%s initialization failed [head=%08x], fudging\n,
+ ring-name, I915_READ_HEAD(ring));
+ I915_WRITE_HEAD(ring, 0);
+ (void)I915_READ_HEAD(ring);
+ }
+
I915_WRITE_CTL(ring,
((ringbuf-size - PAGE_SIZE) RING_NR_PAGES)
| RING_VALID);
--
2.1.0.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: No busy-loop wait_for in the ring init code
On Thu, Aug 07, 2014 at 04:07:59PM +0200, Daniel Vetter wrote: Doing a 1s wait (tops) with the cpu is a bit excessive. Tune it down like everything else in that code. 1s? Didn't wait_for_atomic() take a timeout in microseconds? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Iterate through the initialized DDIs to prepare their buffers
2014-08-05 7:29 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
Not every DDIs is necessarily connected can be strapped off and, in the
future, we'll have platforms with a different number of default DDI
ports. So, let's only call intel_prepare_ddi_buffers() on DDI ports that
are actually detected.
We also use the opportunity to give a struct intel_digital_port to
intel_prepare_ddi_buffers() as we'll need it in a following patch to
query if the port supports HMDI or not.
On my HSW machine this removes the initialization of a couple of
(unused) DDIs.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 5 +
drivers/gpu/drm/i915/intel_ddi.c | 16
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dcf318b..701aae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -176,6 +176,11 @@ enum hpd_pin {
dev-mode_config.encoder_list, \
base.head)
+#define for_each_digital_port(dev, digital_port) \
+ list_for_each_entry(digital_port, \
+ dev-mode_config.encoder_list, \
+ base.base.head)
We can't really assume that every encoder is intel_digital_port since
we still have the CRT encoder on HSW/BDW.
And we can't run this code just for the dig_ports since CRT needs it too.
+
#define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
list_for_each_entry((intel_encoder),
(dev)-mode_config.encoder_list, base.head) \
if ((intel_encoder)-base.crtc == (__crtc))
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index ca1f9a8..fcbddd9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -152,10 +152,12 @@ enum port intel_ddi_get_encoder_port(struct
intel_encoder *intel_encoder)
* in either FDI or DP modes only, as HDMI connections will work with both
* of those
*/
-static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
+static void intel_prepare_ddi_buffers(struct drm_device *dev,
+ struct intel_digital_port
*intel_dig_port)
{
struct drm_i915_private *dev_priv = dev-dev_private;
u32 reg;
+ int port = intel_dig_port-port;
int i, n_hdmi_entries, hdmi_800mV_0dB;
int hdmi_level = dev_priv-vbt.ddi_port_info[port].hdmi_level_shift;
const u32 *ddi_translations_fdi;
@@ -232,13 +234,19 @@ static void intel_prepare_ddi_buffers(struct drm_device
*dev, enum port port)
*/
void intel_prepare_ddi(struct drm_device *dev)
{
- int port;
+ struct intel_digital_port *intel_dig_port;
+ bool visited[I915_MAX_PORTS] = { 0, };
if (!HAS_DDI(dev))
return;
- for (port = PORT_A; port = PORT_E; port++)
- intel_prepare_ddi_buffers(dev, port);
+ for_each_digital_port(dev, intel_dig_port) {
+ if (visited[intel_dig_port-port])
+ continue;
+
+ intel_prepare_ddi_buffers(dev, intel_dig_port);
+ visited[intel_dig_port-port] = true;
+ }
A comment on why we need the visited array is much appreciated,
because it appears to be useless for the code reader. Why is it here?
Will we ever have more than one encoder per port?
}
static const long hsw_ddi_buf_ctl_values[] = {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START
On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote: Ville found an old w/a documented for g4x that suggested that we need to reset the HEAD after writing START. This is a useful fixup for some of the g4x ring initialisation woes, but as usual, not all. Note that a slightly different form of this patch has been used by the reporters of bug76584 without much sucess. (My gm45 works with the old patch - it prints out the debugging information that the fudge was applied.)) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Iterate through the initialized DDIs to prepare their buffers
On Thu, Aug 07, 2014 at 11:17:35AM -0300, Paulo Zanoni wrote:
+#define for_each_digital_port(dev, digital_port) \
+ list_for_each_entry(digital_port, \
+ dev-mode_config.encoder_list, \
+ base.base.head)
We can't really assume that every encoder is intel_digital_port since
we still have the CRT encoder on HSW/BDW.
And we can't run this code just for the dig_ports since CRT needs it too.
Ah, missed that, of course...
+ for_each_digital_port(dev, intel_dig_port) {
+ if (visited[intel_dig_port-port])
+ continue;
+
+ intel_prepare_ddi_buffers(dev, intel_dig_port);
+ visited[intel_dig_port-port] = true;
+ }
A comment on why we need the visited array is much appreciated,
because it appears to be useless for the code reader. Why is it here?
Will we ever have more than one encoder per port?
Because of the MST fake encoder.
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START
On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote:
Ville found an old w/a documented for g4x that suggested that we need to
reset the HEAD after writing START. This is a useful fixup for some of
the g4x ring initialisation woes, but as usual, not all.
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 45e3ec927051..0a37ac0d9c0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring)
* also enforces ordering), otherwise the hw might lose the new ring
* register values. */
I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj));
+
+ /* WaClearRingBufHeadRegAtInit:ctg,elk */
+ if (I915_READ_HEAD(ring)) {
+ DRM_DEBUG(%s initialization failed [head=%08x], fudging\n,
+ ring-name, I915_READ_HEAD(ring));
+ I915_WRITE_HEAD(ring, 0);
+ (void)I915_READ_HEAD(ring);
+ }
Just a clarification that I don't know if this is the right w/a name.
There's no description in the database, but it would seem to fit the
symptoms of HEAD going wild on its own.
The spec says this:
RING_START:
Writing this register also causes the Head Offset to be reset to zero,
and the Wrap Count to be reset to zero.
RING_HEAD:
Ring Buffer Head Offsets must be properly programmed before ring is
enable
So my theory is that the RING_START write actually corruptsa RING_HEAD
instead of clearing it on g4x, and thus we need to rewrite it once more
after writing RING_START, but before we enable the ring.
As far as the patch goes, I think I'd just do the write unconditionally
in case the corruption is somehow magical and doesn't appear until the
ring is started. After all it can't do any harm as the RING_START write
is already supposed to clear it. Not sure if there should be some posting
reads between the steps just for a bit of extra paranoia.
+
I915_WRITE_CTL(ring,
((ringbuf-size - PAGE_SIZE) RING_NR_PAGES)
| RING_VALID);
--
2.1.0.rc1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: read HEAD register back in init_ring_common() to enforce ordering
Withtout this, ring initialization fails reliabily during resume with [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head ff8804 tail start 000e4000 This is not a complete fix, but it is verified to make the ring initialization failures during resume much less likely. We were not able to root-cause this bug (likely HW-specific to Gen4 chips) yet. This is therefore used as a ducttape before problem is fully understood and proper fix created, so that people don't suffer from completely unusable systems in the meantime. The discussion and debugging is happening at https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..7add7ee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -517,6 +517,9 @@ static int init_ring_common(struct intel_engine_cs *ring) else ring_setup_phys_status_page(ring); + /* Enforce ordering by reading HEAD register back */ + I915_READ_HEAD(ring); + /* Initialize the ring. This must happen _after_ we've cleared the ring * registers with the above sequence (the readback of the HEAD registers * also enforces ordering), otherwise the hw might lose the new ring -- Jiri Kosina SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START
On Thu, Aug 07, 2014 at 05:26:20PM +0300, Ville Syrjälä wrote:
On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote:
Ville found an old w/a documented for g4x that suggested that we need to
reset the HEAD after writing START. This is a useful fixup for some of
the g4x ring initialisation woes, but as usual, not all.
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 45e3ec927051..0a37ac0d9c0d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs
*ring)
* also enforces ordering), otherwise the hw might lose the new ring
* register values. */
I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj));
+
+ /* WaClearRingBufHeadRegAtInit:ctg,elk */
+ if (I915_READ_HEAD(ring)) {
+ DRM_DEBUG(%s initialization failed [head=%08x], fudging\n,
+ ring-name, I915_READ_HEAD(ring));
+ I915_WRITE_HEAD(ring, 0);
+ (void)I915_READ_HEAD(ring);
+ }
Just a clarification that I don't know if this is the right w/a name.
There's no description in the database, but it would seem to fit the
symptoms of HEAD going wild on its own.
The spec says this:
RING_START:
Writing this register also causes the Head Offset to be reset to zero,
and the Wrap Count to be reset to zero.
RING_HEAD:
Ring Buffer Head Offsets must be properly programmed before ring is
enable
So my theory is that the RING_START write actually corruptsa RING_HEAD
instead of clearing it on g4x, and thus we need to rewrite it once more
after writing RING_START, but before we enable the ring.
As far as the patch goes, I think I'd just do the write unconditionally
in case the corruption is somehow magical and doesn't appear until the
ring is started. After all it can't do any harm as the RING_START write
is already supposed to clear it. Not sure if there should be some posting
reads between the steps just for a bit of extra paranoia.
The read there is mostly for debugging (it's nice to know when w/a are
being applied after all), but it also helps with ordering. As a
compromise:
/* WaClearRingBufHeadRegAtInit:ctg,elk */
if (I95_READ_HEAD())
DRM_DEBUG();
I915_WRITE_HEAD(ring, 0);
-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] 3.15-rc: regression in suspend
On Thu, Aug 07, 2014 at 02:47:21PM +0200, Jiri Kosina wrote: On Fri, 11 Jul 2014, Pavel Machek wrote: Ok, so I have set up machines for ktest / autobisect, and found out that 3.16-rc1 no longer has that problem. Oh well, bisect would not be fun, anyway... I am still seeing the problem with 3.16-rc2. I'm confused now. Is the bisect result commit 773875bfb6737982903c42d1ee88cf60af80089c Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Jan 27 10:00:30 2014 +0100 drm/i915: Don't set the 8to6 dither flag when not scaling now the culprit or not? Or do we have 2 different bugs at hand here? Three different issues, it seems. Two ring initialization problems, one went away in 3.16 (for me), second did not (suspend for jikos), third -- trivial issue with 8to6 dither. The patch below seems to finally cure the problem at my system; I've just attached it to freedesktop bugzilla, but sending it to this thread as well to hopefully get as much testing coverage by affected people as possible. I am going on with testing whether it really completely fixes the problem or just made it less likely. Picked up for -fixes, thanks for the patch. -Daniel From: Jiri Kosina jkos...@suse.cz Subject: [PATCH] drm/i915: read HEAD register back in init_ring_common() to enforce ordering Withtout this, ring initialization fails reliabily during resume with [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head ff8804 tail start 000e4000 Signed-off-by: Jiri Kosina jkos...@suse.cz --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..7add7ee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -517,6 +517,9 @@ static int init_ring_common(struct intel_engine_cs *ring) else ring_setup_phys_status_page(ring); + /* Enforce ordering by reading HEAD register back */ + I915_READ_HEAD(ring); + /* Initialize the ring. This must happen _after_ we've cleared the ring * registers with the above sequence (the readback of the HEAD registers * also enforces ordering), otherwise the hw might lose the new ring -- Jiri Kosina SUSE Labs ___ 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: Reset the HEAD pointer for the ring after writing START
Ville found an old w/a documented for g4x that suggested that we need to reset the HEAD after writing START. This is a useful fixup for some of the g4x ring initialisation woes, but as usual, not all. v2: Do the rewrite unconditionally anyway References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 45e3ec927051..26ec25afc02a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -537,6 +537,14 @@ static int init_ring_common(struct intel_engine_cs *ring) * also enforces ordering), otherwise the hw might lose the new ring * register values. */ I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); + + /* WaClearRingBufHeadRegAtInit:ctg,elk */ + if (I915_READ_HEAD(ring)) + DRM_DEBUG(%s initialization failed [head=%08x], fudging\n, + ring-name, I915_READ_HEAD(ring)); + I915_WRITE_HEAD(ring, 0); + (void)I915_READ_HEAD(ring); + I915_WRITE_CTL(ring, ((ringbuf-size - PAGE_SIZE) RING_NR_PAGES) | RING_VALID); -- 2.1.0.rc1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] igt_kms: document and export igt_get_property()
On Thu, Aug 07, 2014 at 10:09:05AM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
So we can use it on pm_rpm.c.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
The idea behind the igt_kms_ and kmstest_ prefix split is that kmstests_
are the lower-level helpers wrapping around just the libdrm interfaces
(simalar to the piles of ioctl wrappers we have) and igt_kms_ provides
higher-level code on top.
So I think for this we should use kmstest_ as the prefix.
-Daniel
---
lib/igt_kms.c | 27 +--
lib/igt_kms.h | 4
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 664b9e8..6cca7e8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -592,10 +592,25 @@ static void igt_output_refresh(igt_output_t *output)
display-pipes_in_use |= 1 output-config.pipe;
}
-static bool
-get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
- const char *name, uint32_t *prop_id /* out */,
- uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
+/**
+ * igt_get_property:
+ * @drm_fd: drm file descriptor
+ * @object_id: object whose properties we're going to get
+ * @object_type: type of obj_id (DRM_MODE_OBJECT_*)
+ * @name: name of the property we're going to get
+ * @prop_id: if not NULL, returns the property id
+ * @value: if not NULL, returns the property value
+ * @prop: if not NULL, returns the property, and the caller will have to free
+ *it manually.
+ *
+ * Finds a property with the given name on the given object.
+ *
+ * Returns: true in case we found something.
+ */
+bool
+igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+ const char *name, uint32_t *prop_id /* out */,
+ uint64_t *value /* out */, drmModePropertyPtr *prop /* out */)
{
drmModeObjectPropertiesPtr proplist;
drmModePropertyPtr _prop;
@@ -633,8 +648,8 @@ get_plane_property(int drm_fd, uint32_t plane_id, const
char *name,
uint32_t *prop_id /* out */, uint64_t *value /* out */,
drmModePropertyPtr *prop /* out */)
{
- return get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
- name, prop_id, value, prop);
+ return igt_get_property(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+ name, prop_id, value, prop);
}
static void
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 08b46ab..f8c500e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -255,5 +255,9 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
void igt_enable_connectors(void);
void igt_reset_connectors(void);
+bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
+ const char *name, uint32_t *prop_id, uint64_t *value,
+ drmModePropertyPtr *prop);
+
#endif /* __IGT_KMS_H__ */
--
2.0.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 6/6] igt_kms: add igt_unset_all_crtcs()
On Thu, Aug 07, 2014 at 10:09:07AM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
Both pm_rpm.c and pm_lpsp.c call it disable_all_screens, but let's
give it a name that better describes what the implementation does.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
Same comment as earlier, this should have a kmstest_ prefix and needs to
be grouped together with those functions.
-Daniel
---
lib/igt_kms.c | 18 ++
lib/igt_kms.h | 1 +
tests/pm_lpsp.c | 17 +++--
tests/pm_rpm.c | 8 +---
4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6cca7e8..678b3cd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1560,3 +1560,21 @@ void igt_reset_connectors(void)
close(drm_fd);
}
+
+/**
+ * igt_unset_all_crtcs:
+ * @drm_fd: the DRM fd
+ * @resources: libdrm resources pointer
+ *
+ * Disables all the screens.
+ */
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources)
+{
+ int i, rc;
+
+ for (i = 0; i resources-count_crtcs; i++) {
+ rc = drmModeSetCrtc(drm_fd, resources-crtcs[i], -1, 0, 0, NULL,
+ 0, NULL);
+ igt_assert(rc == 0);
+ }
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f8c500e..43e84f6 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -258,6 +258,7 @@ void igt_reset_connectors(void);
bool igt_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
const char *name, uint32_t *prop_id, uint64_t *value,
drmModePropertyPtr *prop);
+void igt_unset_all_crtcs(int drm_fd, drmModeResPtr resources);
#endif /* __IGT_KMS_H__ */
diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
index 9d3884c..b3a21ea 100644
--- a/tests/pm_lpsp.c
+++ b/tests/pm_lpsp.c
@@ -70,22 +70,11 @@ static bool lpsp_is_enabled(int drm_fd)
return !(val HSW_PWR_WELL_STATE_ENABLED);
}
-static void disable_all_screens(int drm_fd, drmModeResPtr drm_resources)
-{
- int i, rc;
-
- for (i = 0; i drm_resources-count_crtcs; i++) {
- rc = drmModeSetCrtc(drm_fd, drm_resources-crtcs[i], -1, 0, 0,
- NULL, 0, NULL);
- igt_assert(rc == 0);
- }
-}
-
/* The LPSP mode is all about an enabled pipe, but we expect to also be in
the
* low power mode when no pipes are enabled, so do this check anyway. */
static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
{
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
igt_assert(lpsp_is_enabled(drm_fd));
}
@@ -131,7 +120,7 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
.name = Custom 1024x768,
};
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
for (i = 0; i drm_res-count_connectors; i++) {
drmModeConnectorPtr c = drm_connectors[i];
@@ -193,7 +182,7 @@ static void non_edp_subtest(int drm_fd, drmModeResPtr
drm_res,
uint32_t connector_id = 0, crtc_id = 0, buffer_id = 0;
drmModeModeInfoPtr mode = NULL;
- disable_all_screens(drm_fd, drm_res);
+ igt_unset_all_crtcs(drm_fd, drm_res);
for (i = 0; i drm_res-count_connectors; i++) {
drmModeConnectorPtr c = drm_connectors[i];
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 800ab4b..84c71bd 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -232,13 +232,7 @@ static void disable_all_screens_dpms(struct
mode_set_data *data)
static void disable_all_screens(struct mode_set_data *data)
{
- int i, rc;
-
- for (i = 0; i data-res-count_crtcs; i++) {
- rc = drmModeSetCrtc(drm_fd, data-res-crtcs[i], -1, 0, 0,
- NULL, 0, NULL);
- igt_assert(rc == 0);
- }
+ igt_unset_all_crtcs(drm_fd, data-res);
}
static struct scanout_fb *create_fb(struct mode_set_data *data, int width,
--
2.0.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: read HEAD register back in init_ring_common() to enforce ordering
On Thu, Aug 07, 2014 at 04:29:53PM +0200, Jiri Kosina wrote: Withtout this, ring initialization fails reliabily during resume with [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head ff8804 tail start 000e4000 This is not a complete fix, but it is verified to make the ring initialization failures during resume much less likely. We were not able to root-cause this bug (likely HW-specific to Gen4 chips) yet. This is therefore used as a ducttape before problem is fully understood and proper fix created, so that people don't suffer from completely unusable systems in the meantime. The discussion and debugging is happening at https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Jiri Kosina jkos...@suse.cz Ok, picked this one here, has the nicer commit message ;-) -Daniel --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..7add7ee 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -517,6 +517,9 @@ static int init_ring_common(struct intel_engine_cs *ring) else ring_setup_phys_status_page(ring); + /* Enforce ordering by reading HEAD register back */ + I915_READ_HEAD(ring); + /* Initialize the ring. This must happen _after_ we've cleared the ring * registers with the above sequence (the readback of the HEAD registers * also enforces ordering), otherwise the hw might lose the new ring -- Jiri Kosina SUSE Labs -- 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: Restore drm_file-is_master
Hi
On Thu, Aug 7, 2014 at 3:04 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
Despite the claims of
commit 48ba813701eb14b3008edefef4a0789b328e278c
Author: David Herrmann dh.herrm...@gmail.com
Date: Tue Jul 22 18:46:09 2014 +0200
drm: drop redundant drm_file-is_master
drm_file-is_master is not synomous with having drm_file-master ==
drm_file-minor-master. This is because drm_file-master is the same
for all drm_files of the same generation and so when there is a master,
every drm_file believes itself to be the master. Confusion ensues and
things go pear shaped when one file is closed and there is no master
anymore.
Uagh, embarrassing. A revert is fine with me, but I'll try to review
your patch once I get home.
Thanks
David
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82283
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Alex Deucher alexander.deuc...@amd.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: David Herrmann dh.herrm...@gmail.com
---
drivers/gpu/drm/drm_drv.c | 5 ++---
drivers/gpu/drm/drm_fops.c | 12 ++--
include/drm/drmP.h | 3 ++-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a1863d8..76cdb51 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -198,6 +198,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void
*data,
if (unlikely(ret != 0))
drm_master_put(file_priv-minor-master);
}
+ file_priv-is_master = ret == 0;
out_unlock:
mutex_unlock(dev-master_mutex);
@@ -213,13 +214,11 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void
*data,
if (!drm_is_master(file_priv))
goto out_unlock;
- if (!file_priv-minor-master)
- goto out_unlock;
-
ret = 0;
if (dev-driver-master_drop)
dev-driver-master_drop(dev, file_priv, false);
drm_master_put(file_priv-minor-master);
+ file_priv-is_master = false;
out_unlock:
mutex_unlock(dev-master_mutex);
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index c0166ba..083f7b9 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -196,6 +196,7 @@ static int drm_open_helper(struct file *filp, struct
drm_minor *minor)
/* take another reference for the copy in the local file priv
*/
priv-master = drm_master_get(priv-minor-master);
+ priv-is_master = true;
priv-authenticated = 1;
if (dev-driver-master_create) {
@@ -434,12 +435,11 @@ int drm_release(struct inode *inode, struct file *filp)
}
mutex_unlock(dev-struct_mutex);
- if (file_priv-minor-master == file_priv-master) {
- /* drop the reference held my the minor */
- if (dev-driver-master_drop)
- dev-driver-master_drop(dev, file_priv,
true);
- drm_master_put(file_priv-minor-master);
- }
+ /* drop the reference held my the minor */
+ if (dev-driver-master_drop)
+ dev-driver-master_drop(dev, file_priv, true);
+ drm_master_put(file_priv-minor-master);
+ file_priv-is_master = false;
}
/* drop the master reference held by the file priv */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0ffce5c..70a6598 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -378,6 +378,7 @@ struct drm_prime_file_private {
/** File private data */
struct drm_file {
+ bool is_master:1;
unsigned authenticated :1;
/* true when the client has asked us to expose stereo 3D mode flags */
unsigned stereo_allowed :1;
@@ -1177,7 +1178,7 @@ static inline bool drm_is_primary_client(const struct
drm_file *file_priv)
*/
static inline bool drm_is_master(const struct drm_file *file)
{
- return file-master file-master == file-minor-master;
+ return file-is_master;
}
/**/
--
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: Reset the HEAD pointer for the ring after writing START
On Thu, Aug 07, 2014 at 03:39:54PM +0100, Chris Wilson wrote: Ville found an old w/a documented for g4x that suggested that we need to reset the HEAD after writing START. This is a useful fixup for some of the g4x ring initialisation woes, but as usual, not all. v2: Do the rewrite unconditionally anyway References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com Yeah looks sensible to me. So based on my gut feeling just from the seeing the w/a name this is: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 45e3ec927051..26ec25afc02a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -537,6 +537,14 @@ static int init_ring_common(struct intel_engine_cs *ring) * also enforces ordering), otherwise the hw might lose the new ring * register values. */ I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); + + /* WaClearRingBufHeadRegAtInit:ctg,elk */ + if (I915_READ_HEAD(ring)) + DRM_DEBUG(%s initialization failed [head=%08x], fudging\n, + ring-name, I915_READ_HEAD(ring)); + I915_WRITE_HEAD(ring, 0); + (void)I915_READ_HEAD(ring); + I915_WRITE_CTL(ring, ((ringbuf-size - PAGE_SIZE) RING_NR_PAGES) | RING_VALID); -- 2.1.0.rc1 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
I tested here on HSW a full sw nuke/cache clean and I didn't liked the
result.
It seems to compress less than the hw one and to recompress everything a
lot and stay less time compressed.
So, imho v3 is the way to go.
On Mon, Aug 4, 2014 at 3:51 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote:
According to spec FBC on BDW and HSW are identical without any gaps.
So let's copy the nuke and let FBC really start compressing stuff.
Without this patch we can verify with false color that nothing is being
compressed. With the nuke in place and false color it is possible
to see false color debugs.
Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on
LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer
rend
cache would never been cleaned and FBC would stop compressing buffer.
One alternative is to cache clean on software frontbuffer tracking.
v2: Fix rebase conflict.
v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c| 3 +++
drivers/gpu/drm/i915/intel_pm.c | 10 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +-
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 2a372f2..25d7365 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct
drm_device *dev,
extern void i915_redisable_vga(struct drm_device *dev);
extern void i915_redisable_vga_power_on(struct drm_device *dev);
extern bool intel_fbc_enabled(struct drm_device *dev);
+extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
extern void intel_disable_fbc(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 883af0b..c8421cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
intel_edp_psr_flush(dev, frontbuffer_bits);
+
+ if (IS_GEN8(dev))
+ gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 684dc5f..de07d3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv-display.fbc_enabled(dev);
}
+void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!IS_GEN8(dev))
+ return;
+
+ I915_WRITE(MSG_FBC_REND_STATE, value);
+}
+
static void intel_fbc_work_fn(struct work_struct *__work)
{
struct intel_fbc_work *work =
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2908896..2fe871c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
{
u32 flags = 0;
u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES;
+ int ret;
flags |= PIPE_CONTROL_CS_STALL;
@@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
}
- return gen8_emit_pipe_control(ring, flags, scratch_addr);
+ ret = gen8_emit_pipe_control(ring, flags, scratch_addr);
+ if (ret)
+ return ret;
+
+ if (!invalidate_domains flush_domains)
+ return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
+ return 0;
}
static void ring_write_tail(struct intel_engine_cs *ring,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW
The rest of the series was a reference for the records of what I had and let semaphores on bdw a bit more stable. But even with them we still get hungs so please consider only to get the revert for now. On Mon, Aug 4, 2014 at 11:15 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote: This reverts commit 521e62e49a42661a4ee0102644517dbe2f100a23. Although POST_SYNC brought a bit of stability to Semaphores on BDW it didn't solved all issues and some hungs can still occour when semaphores are enabled on BDW. Also some sloweness can be found on some igt tests, althoguth it apparently doesn't affect real workloads. Besides that, no real performance gain was found on our tests with different and even multiple workloads. Let's disable it again for now. At least until we are sure it is safe to re-enable it. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..ec96f9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -481,6 +481,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) if (i915.semaphores = 0) return i915.semaphores; + /* Until we get further testing... */ + if (IS_GEN8(dev)) + return false; + #ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Restore drm_file-is_master
On 8 August 2014 02:00, David Herrmann dh.herrm...@gmail.com wrote: Hi On Thu, Aug 7, 2014 at 3:04 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: Despite the claims of commit 48ba813701eb14b3008edefef4a0789b328e278c Author: David Herrmann dh.herrm...@gmail.com Date: Tue Jul 22 18:46:09 2014 +0200 drm: drop redundant drm_file-is_master drm_file-is_master is not synomous with having drm_file-master == drm_file-minor-master. This is because drm_file-master is the same for all drm_files of the same generation and so when there is a master, every drm_file believes itself to be the master. Confusion ensues and things go pear shaped when one file is closed and there is no master anymore. Uagh, embarrassing. A revert is fine with me, but I'll try to review your patch once I get home. At this point I'll just revert, though I do like the wrapper instead of checking the flag, but its late in the day. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
