* [PATCH 01/15] drm/i915: Bring UP Power Wells before disabling RC6.
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 02/15] drm/i915: Don't save/restore RS when not used Rodrigo Vivi
` (13 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Deepak S, Paulo Zanoni, Rodrigo Vivi
From: Deepak S <deepak.s@intel.com>
We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.
v2: updated commit message. (Daniel)
Reviewer: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3f88f29..d3085b7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3407,8 +3407,14 @@ static void valleyview_disable_rps(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ /* we're doing forcewake before Disabling RC6,
+ * This what the BIOS expects when going into suspend */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
gen6_disable_rps_interrupts(dev);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 02/15] drm/i915: Don't save/restore RS when not used
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 01/15] drm/i915: Bring UP Power Wells before disabling RC6 Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO Rodrigo Vivi
` (12 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Ben Widawsky, Ben Widawsky
From: Ben Widawsky <benjamin.widawsky@intel.com>
v2: fix conflict on rebase.
Cc: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 3b99390..15ec7e4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -563,6 +563,7 @@ mi_set_context(struct intel_engine_cs *ring,
struct intel_context *new_context,
u32 hw_flags)
{
+ u32 flags = hw_flags | MI_MM_SPACE_GTT;
int ret;
/* w/a: If Flush TLB Invalidation Mode is enabled, driver must do a TLB
@@ -576,6 +577,10 @@ mi_set_context(struct intel_engine_cs *ring,
return ret;
}
+ /* These flags are for resource streamer on HSW+ */
+ if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
+ flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -589,10 +594,7 @@ mi_set_context(struct intel_engine_cs *ring,
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_SET_CONTEXT);
intel_ring_emit(ring, i915_gem_obj_ggtt_offset(new_context->legacy_hw_ctx.rcs_state) |
- MI_MM_SPACE_GTT |
- MI_SAVE_EXT_STATE_EN |
- MI_RESTORE_EXT_STATE_EN |
- hw_flags);
+ flags);
/*
* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
* WaMiSetContext_Hang:snb,ivb,vlv
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 01/15] drm/i915: Bring UP Power Wells before disabling RC6 Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 02/15] drm/i915: Don't save/restore RS when not used Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 7:56 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 04/15] drm/i915: honour forced connector modes Rodrigo Vivi
` (11 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.
This should prevent UXA from getting stuck in a null render loop after a
failed resume.
v2 (Rodrigo): Fix conflict and add VCS2 ring and
s/intel_ring_buffer/intel_engine_cs.
Reported-by: Jiri Kosina <jikos@jikos.cz>
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 60998fc..288ff61 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1233,6 +1233,19 @@ eb_get_batch(struct eb_vmas *eb)
return vma->obj;
}
+static bool
+intel_ring_valid(struct intel_engine_cs *ring)
+{
+ switch (ring->id) {
+ case RCS: return true;
+ case VCS: return HAS_BSD(ring->dev);
+ case BCS: return HAS_BLT(ring->dev);
+ case VECS: return HAS_VEBOX(ring->dev);
+ case VCS2: return HAS_BSD2(ring->dev);
+ default: return false;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1289,7 +1302,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!intel_ring_initialized(ring)) {
DRM_DEBUG("execbuf with invalid ring: %d\n",
(int)(args->flags & I915_EXEC_RING_MASK));
- return -EINVAL;
+ return intel_ring_valid(ring) ? -EIO : -EINVAL;
}
if (args->buffer_count < 1) {
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-05 14:51 ` [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO Rodrigo Vivi
@ 2014-08-06 7:56 ` Daniel Vetter
2014-08-06 8:12 ` Chris Wilson
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 7:56 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> If we try to execute on a known ring, but it has failed to be
> initialised correctly, report that the GPU is hung rather than the
> command invalid. This leaves us reporting EINVAL only if the user
> requests execution on a ring that is not supported by the device.
>
> This should prevent UXA from getting stuck in a null render loop after a
> failed resume.
>
> v2 (Rodrigo): Fix conflict and add VCS2 ring and
> s/intel_ring_buffer/intel_engine_cs.
>
> Reported-by: Jiri Kosina <jikos@jikos.cz>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
This isn't required any more, see
commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Wed Apr 9 09:19:43 2014 +0100
drm/i915: Mark device as wedged if we fail to resume
for the alternate merged patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 60998fc..288ff61 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1233,6 +1233,19 @@ eb_get_batch(struct eb_vmas *eb)
> return vma->obj;
> }
>
> +static bool
> +intel_ring_valid(struct intel_engine_cs *ring)
> +{
> + switch (ring->id) {
> + case RCS: return true;
> + case VCS: return HAS_BSD(ring->dev);
> + case BCS: return HAS_BLT(ring->dev);
> + case VECS: return HAS_VEBOX(ring->dev);
> + case VCS2: return HAS_BSD2(ring->dev);
> + default: return false;
> + }
> +}
> +
> static int
> i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> struct drm_file *file,
> @@ -1289,7 +1302,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> if (!intel_ring_initialized(ring)) {
> DRM_DEBUG("execbuf with invalid ring: %d\n",
> (int)(args->flags & I915_EXEC_RING_MASK));
> - return -EINVAL;
> + return intel_ring_valid(ring) ? -EIO : -EINVAL;
> }
>
> if (args->buffer_count < 1) {
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-06 7:56 ` Daniel Vetter
@ 2014-08-06 8:12 ` Chris Wilson
2014-08-06 8:39 ` Daniel Vetter
0 siblings, 1 reply; 36+ messages in thread
From: Chris Wilson @ 2014-08-06 8:12 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi
On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote:
> On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > If we try to execute on a known ring, but it has failed to be
> > initialised correctly, report that the GPU is hung rather than the
> > command invalid. This leaves us reporting EINVAL only if the user
> > requests execution on a ring that is not supported by the device.
> >
> > This should prevent UXA from getting stuck in a null render loop after a
> > failed resume.
> >
> > v2 (Rodrigo): Fix conflict and add VCS2 ring and
> > s/intel_ring_buffer/intel_engine_cs.
> >
> > Reported-by: Jiri Kosina <jikos@jikos.cz>
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> This isn't required any more, see
>
> commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Wed Apr 9 09:19:43 2014 +0100
>
> drm/i915: Mark device as wedged if we fail to resume
>
> for the alternate merged patch.
Hmm, there is still a path that ends here, but the example above is
already fixed as you say.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-06 8:12 ` Chris Wilson
@ 2014-08-06 8:39 ` Daniel Vetter
2014-08-08 9:17 ` Chris Wilson
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:39 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, Rodrigo Vivi, intel-gfx
On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote:
> On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote:
> > On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > If we try to execute on a known ring, but it has failed to be
> > > initialised correctly, report that the GPU is hung rather than the
> > > command invalid. This leaves us reporting EINVAL only if the user
> > > requests execution on a ring that is not supported by the device.
> > >
> > > This should prevent UXA from getting stuck in a null render loop after a
> > > failed resume.
> > >
> > > v2 (Rodrigo): Fix conflict and add VCS2 ring and
> > > s/intel_ring_buffer/intel_engine_cs.
> > >
> > > Reported-by: Jiri Kosina <jikos@jikos.cz>
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > This isn't required any more, see
> >
> > commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date: Wed Apr 9 09:19:43 2014 +0100
> >
> > drm/i915: Mark device as wedged if we fail to resume
> >
> > for the alternate merged patch.
>
> Hmm, there is still a path that ends here, but the example above is
> already fixed as you say.
We have the EIO check both in the resume and driver load paths. Which
other path are we missing?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-06 8:39 ` Daniel Vetter
@ 2014-08-08 9:17 ` Chris Wilson
2014-08-08 9:46 ` Daniel Vetter
0 siblings, 1 reply; 36+ messages in thread
From: Chris Wilson @ 2014-08-08 9:17 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi
On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote:
> On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote:
> > On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote:
> > > On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> > > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > >
> > > > If we try to execute on a known ring, but it has failed to be
> > > > initialised correctly, report that the GPU is hung rather than the
> > > > command invalid. This leaves us reporting EINVAL only if the user
> > > > requests execution on a ring that is not supported by the device.
> > > >
> > > > This should prevent UXA from getting stuck in a null render loop after a
> > > > failed resume.
> > > >
> > > > v2 (Rodrigo): Fix conflict and add VCS2 ring and
> > > > s/intel_ring_buffer/intel_engine_cs.
> > > >
> > > > Reported-by: Jiri Kosina <jikos@jikos.cz>
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >
> > > This isn't required any more, see
> > >
> > > commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date: Wed Apr 9 09:19:43 2014 +0100
> > >
> > > drm/i915: Mark device as wedged if we fail to resume
> > >
> > > for the alternate merged patch.
> >
> > Hmm, there is still a path that ends here, but the example above is
> > already fixed as you say.
>
> We have the EIO check both in the resume and driver load paths. Which
> other path are we missing?
The GPU may be set to wedged, but this check in execbuffer occurs before
we check for a wedged GPU.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-08 9:17 ` Chris Wilson
@ 2014-08-08 9:46 ` Daniel Vetter
2014-08-08 9:52 ` Chris Wilson
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Vetter @ 2014-08-08 9:46 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, Rodrigo Vivi, intel-gfx
On Fri, Aug 08, 2014 at 10:17:10AM +0100, Chris Wilson wrote:
> On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote:
> > On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote:
> > > On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote:
> > > > On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> > > > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > > >
> > > > > If we try to execute on a known ring, but it has failed to be
> > > > > initialised correctly, report that the GPU is hung rather than the
> > > > > command invalid. This leaves us reporting EINVAL only if the user
> > > > > requests execution on a ring that is not supported by the device.
> > > > >
> > > > > This should prevent UXA from getting stuck in a null render loop after a
> > > > > failed resume.
> > > > >
> > > > > v2 (Rodrigo): Fix conflict and add VCS2 ring and
> > > > > s/intel_ring_buffer/intel_engine_cs.
> > > > >
> > > > > Reported-by: Jiri Kosina <jikos@jikos.cz>
> > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > >
> > > > This isn't required any more, see
> > > >
> > > > commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
> > > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Date: Wed Apr 9 09:19:43 2014 +0100
> > > >
> > > > drm/i915: Mark device as wedged if we fail to resume
> > > >
> > > > for the alternate merged patch.
> > >
> > > Hmm, there is still a path that ends here, but the example above is
> > > already fixed as you say.
> >
> > We have the EIO check both in the resume and driver load paths. Which
> > other path are we missing?
>
> The GPU may be set to wedged, but this check in execbuffer occurs before
> we check for a wedged GPU.
But we no longer free the ring structures over suspedn/resume, so at least
the commit message is outdated.
I wonder whether the easier fix wouldn't be to continue ring init if we
get an -EIO.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-08-08 9:46 ` Daniel Vetter
@ 2014-08-08 9:52 ` Chris Wilson
0 siblings, 0 replies; 36+ messages in thread
From: Chris Wilson @ 2014-08-08 9:52 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi
On Fri, Aug 08, 2014 at 11:46:07AM +0200, Daniel Vetter wrote:
> On Fri, Aug 08, 2014 at 10:17:10AM +0100, Chris Wilson wrote:
> > On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote:
> > > On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote:
> > > > On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote:
> > > > > On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote:
> > > > > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > >
> > > > > > If we try to execute on a known ring, but it has failed to be
> > > > > > initialised correctly, report that the GPU is hung rather than the
> > > > > > command invalid. This leaves us reporting EINVAL only if the user
> > > > > > requests execution on a ring that is not supported by the device.
> > > > > >
> > > > > > This should prevent UXA from getting stuck in a null render loop after a
> > > > > > failed resume.
> > > > > >
> > > > > > v2 (Rodrigo): Fix conflict and add VCS2 ring and
> > > > > > s/intel_ring_buffer/intel_engine_cs.
> > > > > >
> > > > > > Reported-by: Jiri Kosina <jikos@jikos.cz>
> > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > >
> > > > > This isn't required any more, see
> > > > >
> > > > > commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0
> > > > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Date: Wed Apr 9 09:19:43 2014 +0100
> > > > >
> > > > > drm/i915: Mark device as wedged if we fail to resume
> > > > >
> > > > > for the alternate merged patch.
> > > >
> > > > Hmm, there is still a path that ends here, but the example above is
> > > > already fixed as you say.
> > >
> > > We have the EIO check both in the resume and driver load paths. Which
> > > other path are we missing?
> >
> > The GPU may be set to wedged, but this check in execbuffer occurs before
> > we check for a wedged GPU.
>
> But we no longer free the ring structures over suspedn/resume, so at least
> the commit message is outdated.
Went off on an incorrect tangent.
> I wonder whether the easier fix wouldn't be to continue ring init if we
> get an -EIO.
Right, that's the problem I remember. But I am not sure if it is really
worth it. It's only to hide log spew in a corner case of a corner case in
UXA.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 04/15] drm/i915: honour forced connector modes
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (2 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:15 ` Daniel Vetter
2014-08-06 15:00 ` Jesse Barnes
2014-08-05 14:51 ` [PATCH 05/15] drm/i915: Don't promote UC to WT automagically Rodrigo Vivi
` (10 subsequent siblings)
14 siblings, 2 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off. Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config that tries to enable a connector (disabling is easy!).
Based on earlier patches by Jesse Barnes.
v2: Remove Jesse's patch
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_fbdev.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index f475414..5d879d18 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
int num_connectors_enabled = 0;
int num_connectors_detected = 0;
- /*
- * If the user specified any force options, just bail here
- * and use that config.
- */
- for (i = 0; i < fb_helper->connector_count; i++) {
- struct drm_fb_helper_connector *fb_conn;
- struct drm_connector *connector;
-
- fb_conn = fb_helper->connector_info[i];
- connector = fb_conn->connector;
-
- if (!enabled[i])
- continue;
-
- if (connector->force != DRM_FORCE_UNSPECIFIED)
- return false;
- }
-
save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
GFP_KERNEL);
if (!save_enabled)
@@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
continue;
}
+ if (connector->force == DRM_FORCE_OFF) {
+ DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
+ connector->name);
+ enabled[i] = false;
+ continue;
+ }
+
encoder = connector->encoder;
if (!encoder || WARN_ON(!encoder->crtc)) {
+ if (connector->force > DRM_FORCE_OFF)
+ goto bail;
+
DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
connector->name);
enabled[i] = false;
@@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
for (j = 0; j < fb_helper->connector_count; j++) {
if (crtcs[j] == new_crtc) {
DRM_DEBUG_KMS("fallback: cloned configuration\n");
- fallback = true;
- goto out;
+ goto bail;
}
}
@@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
fallback = true;
}
-out:
if (fallback) {
+bail:
DRM_DEBUG_KMS("Not using firmware configuration\n");
memcpy(enabled, save_enabled, dev->mode_config.num_connector);
kfree(save_enabled);
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 04/15] drm/i915: honour forced connector modes
2014-08-05 14:51 ` [PATCH 04/15] drm/i915: honour forced connector modes Rodrigo Vivi
@ 2014-08-06 8:15 ` Daniel Vetter
2014-08-06 15:00 ` Jesse Barnes
1 sibling, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:15 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:15AM -0700, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> In the move over to use BIOS connector configs, we lost the ability to
> force a specific set of connectors on or off. Try to remedy that by
> dropping back to the old behavior if we detect a hard coded connector
> config that tries to enable a connector (disabling is easy!).
>
> Based on earlier patches by Jesse Barnes.
>
> v2: Remove Jesse's patch
>
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Queued for -next, thanks for the patch. I've also pulled in the drm
cmdline mode parsing rework. Is that all that's been straggling wrt our
own initial config juggling?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 33 ++++++++++++---------------------
> 1 file changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f475414..5d879d18 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> int num_connectors_enabled = 0;
> int num_connectors_detected = 0;
>
> - /*
> - * If the user specified any force options, just bail here
> - * and use that config.
> - */
> - for (i = 0; i < fb_helper->connector_count; i++) {
> - struct drm_fb_helper_connector *fb_conn;
> - struct drm_connector *connector;
> -
> - fb_conn = fb_helper->connector_info[i];
> - connector = fb_conn->connector;
> -
> - if (!enabled[i])
> - continue;
> -
> - if (connector->force != DRM_FORCE_UNSPECIFIED)
> - return false;
> - }
> -
> save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
> GFP_KERNEL);
> if (!save_enabled)
> @@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> continue;
> }
>
> + if (connector->force == DRM_FORCE_OFF) {
> + DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
> + connector->name);
> + enabled[i] = false;
> + continue;
> + }
> +
> encoder = connector->encoder;
> if (!encoder || WARN_ON(!encoder->crtc)) {
> + if (connector->force > DRM_FORCE_OFF)
> + goto bail;
> +
> DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
> connector->name);
> enabled[i] = false;
> @@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> for (j = 0; j < fb_helper->connector_count; j++) {
> if (crtcs[j] == new_crtc) {
> DRM_DEBUG_KMS("fallback: cloned configuration\n");
> - fallback = true;
> - goto out;
> + goto bail;
> }
> }
>
> @@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> fallback = true;
> }
>
> -out:
> if (fallback) {
> +bail:
> DRM_DEBUG_KMS("Not using firmware configuration\n");
> memcpy(enabled, save_enabled, dev->mode_config.num_connector);
> kfree(save_enabled);
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 04/15] drm/i915: honour forced connector modes
2014-08-05 14:51 ` [PATCH 04/15] drm/i915: honour forced connector modes Rodrigo Vivi
2014-08-06 8:15 ` Daniel Vetter
@ 2014-08-06 15:00 ` Jesse Barnes
1 sibling, 0 replies; 36+ messages in thread
From: Jesse Barnes @ 2014-08-06 15:00 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, 5 Aug 2014 07:51:15 -0700
Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> In the move over to use BIOS connector configs, we lost the ability to
> force a specific set of connectors on or off. Try to remedy that by
> dropping back to the old behavior if we detect a hard coded connector
> config that tries to enable a connector (disabling is easy!).
>
> Based on earlier patches by Jesse Barnes.
>
> v2: Remove Jesse's patch
>
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 33 ++++++++++++---------------------
> 1 file changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f475414..5d879d18 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> int num_connectors_enabled = 0;
> int num_connectors_detected = 0;
>
> - /*
> - * If the user specified any force options, just bail here
> - * and use that config.
> - */
> - for (i = 0; i < fb_helper->connector_count; i++) {
> - struct drm_fb_helper_connector *fb_conn;
> - struct drm_connector *connector;
> -
> - fb_conn = fb_helper->connector_info[i];
> - connector = fb_conn->connector;
> -
> - if (!enabled[i])
> - continue;
> -
> - if (connector->force != DRM_FORCE_UNSPECIFIED)
> - return false;
> - }
> -
> save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
> GFP_KERNEL);
> if (!save_enabled)
> @@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> continue;
> }
>
> + if (connector->force == DRM_FORCE_OFF) {
> + DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
> + connector->name);
> + enabled[i] = false;
> + continue;
> + }
> +
> encoder = connector->encoder;
> if (!encoder || WARN_ON(!encoder->crtc)) {
> + if (connector->force > DRM_FORCE_OFF)
> + goto bail;
> +
> DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
> connector->name);
> enabled[i] = false;
> @@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> for (j = 0; j < fb_helper->connector_count; j++) {
> if (crtcs[j] == new_crtc) {
> DRM_DEBUG_KMS("fallback: cloned configuration\n");
> - fallback = true;
> - goto out;
> + goto bail;
> }
> }
>
> @@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> fallback = true;
> }
>
> -out:
> if (fallback) {
> +bail:
> DRM_DEBUG_KMS("Not using firmware configuration\n");
> memcpy(enabled, save_enabled, dev->mode_config.num_connector);
> kfree(save_enabled);
Could have sworn I'd already R-b'd this one, but maybe that was the
other one to read the force config earlier. Anyway:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 05/15] drm/i915: Don't promote UC to WT automagically
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (3 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 04/15] drm/i915: honour forced connector modes Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 7:57 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 06/15] drm/i915: Refactor the physical and virtual page hws setup Rodrigo Vivi
` (9 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
If the object is already UC leave it as UC instead of automagically
promoting it to WT in i915_gem_object_pin_to_display_plane() when
the hardware is WT capable.
Supposedly the user wanted UC for a reason, so let's respect that.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dcd8d7b..5710571 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3840,6 +3840,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
struct intel_engine_cs *pipelined)
{
u32 old_read_domains, old_write_domain;
+ unsigned int cache_level;
bool was_pin_display;
int ret;
@@ -3864,8 +3865,12 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
* of uncaching, which would allow us to flush all the LLC-cached data
* with that bit in the PTE to main memory with just one PIPE_CONTROL.
*/
- ret = i915_gem_object_set_cache_level(obj,
- HAS_WT(obj->base.dev) ? I915_CACHE_WT : I915_CACHE_NONE);
+ if (HAS_WT(obj->base.dev) && obj->cache_level != I915_CACHE_NONE)
+ cache_level = I915_CACHE_WT;
+ else
+ cache_level = I915_CACHE_NONE;
+
+ ret = i915_gem_object_set_cache_level(obj, cache_level);
if (ret)
goto err_unpin_display;
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 05/15] drm/i915: Don't promote UC to WT automagically
2014-08-05 14:51 ` [PATCH 05/15] drm/i915: Don't promote UC to WT automagically Rodrigo Vivi
@ 2014-08-06 7:57 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 7:57 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:16AM -0700, Rodrigo Vivi wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If the object is already UC leave it as UC instead of automagically
> promoting it to WT in i915_gem_object_pin_to_display_plane() when
> the hardware is WT capable.
>
> Supposedly the user wanted UC for a reason, so let's respect that.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
We don't have a use case for this, so can be dropped.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index dcd8d7b..5710571 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3840,6 +3840,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> struct intel_engine_cs *pipelined)
> {
> u32 old_read_domains, old_write_domain;
> + unsigned int cache_level;
> bool was_pin_display;
> int ret;
>
> @@ -3864,8 +3865,12 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> * of uncaching, which would allow us to flush all the LLC-cached data
> * with that bit in the PTE to main memory with just one PIPE_CONTROL.
> */
> - ret = i915_gem_object_set_cache_level(obj,
> - HAS_WT(obj->base.dev) ? I915_CACHE_WT : I915_CACHE_NONE);
> + if (HAS_WT(obj->base.dev) && obj->cache_level != I915_CACHE_NONE)
> + cache_level = I915_CACHE_WT;
> + else
> + cache_level = I915_CACHE_NONE;
> +
> + ret = i915_gem_object_set_cache_level(obj, cache_level);
> if (ret)
> goto err_unpin_display;
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 06/15] drm/i915: Refactor the physical and virtual page hws setup
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (4 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 05/15] drm/i915: Don't promote UC to WT automagically Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:17 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 07/15] drm/i915: clean up PPGTT checking logic Rodrigo Vivi
` (8 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
We duplicated the legacy physical HWS setup routine for no good reason.
Combine it with the more recent virtual HWS setup for simplicity.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_dma.c | 16 +------
drivers/gpu/drm/i915/intel_ringbuffer.c | 81 ++++++++++++++++-----------------
2 files changed, 39 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03a..f76d2bf 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -104,17 +104,6 @@ void i915_update_dri1_breadcrumb(struct drm_device *dev)
}
}
-static void i915_write_hws_pga(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 addr;
-
- addr = dev_priv->status_page_dmah->busaddr;
- if (INTEL_INFO(dev)->gen >= 4)
- addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
- I915_WRITE(HWS_PGA, addr);
-}
-
/**
* Frees the hardware status page, whether it's a physical address or a virtual
* address set up by the X Server.
@@ -255,10 +244,7 @@ static int i915_dma_resume(struct drm_device *dev)
}
DRM_DEBUG_DRIVER("hw status page @ %p\n",
ring->status_page.page_addr);
- if (ring->status_page.gfx_addr != 0)
- intel_ring_setup_status_page(ring);
- else
- i915_write_hws_pga(dev);
+ intel_ring_setup_status_page(ring);
DRM_DEBUG_DRIVER("Enabled hardware status page\n");
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b3d8f76..b7894d1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -443,17 +443,6 @@ u64 intel_ring_get_active_head(struct intel_engine_cs *ring)
return acthd;
}
-static void ring_setup_phys_status_page(struct intel_engine_cs *ring)
-{
- struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 addr;
-
- addr = dev_priv->status_page_dmah->busaddr;
- if (INTEL_INFO(ring->dev)->gen >= 4)
- addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
- I915_WRITE(HWS_PGA, addr);
-}
-
static bool stop_ring(struct intel_engine_cs *ring)
{
struct drm_i915_private *dev_priv = to_i915(ring->dev);
@@ -511,10 +500,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
}
}
- if (I915_NEED_GFX_HWS(dev))
- intel_ring_setup_status_page(ring);
- else
- ring_setup_phys_status_page(ring);
+ intel_ring_setup_status_page(ring);
/* Initialize the ring. This must happen _after_ we've cleared the ring
* registers with the above sequence (the readback of the HEAD registers
@@ -1101,39 +1087,48 @@ void intel_ring_setup_status_page(struct intel_engine_cs *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 mmio = 0;
+ u32 mmio, addr;
- /* The ring status page addresses are no longer next to the rest of
- * the ring registers as of gen7.
- */
- if (IS_GEN7(dev)) {
- switch (ring->id) {
- case RCS:
- mmio = RENDER_HWS_PGA_GEN7;
- break;
- case BCS:
- mmio = BLT_HWS_PGA_GEN7;
- break;
- /*
- * VCS2 actually doesn't exist on Gen7. Only shut up
- * gcc switch check warning
+ if (!I915_NEED_GFX_HWS(dev)) {
+ addr = dev_priv->status_page_dmah->busaddr;
+ if (INTEL_INFO(ring->dev)->gen >= 4)
+ addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
+ mmio = HWS_PGA;
+ } else {
+ addr = ring->status_page.gfx_addr;
+ /* The ring status page addresses are no longer next to the rest of
+ * the ring registers as of gen7.
*/
- case VCS2:
- case VCS:
- mmio = BSD_HWS_PGA_GEN7;
- break;
- case VECS:
- mmio = VEBOX_HWS_PGA_GEN7;
- break;
+ if (IS_GEN7(dev)) {
+ switch (ring->id) {
+ default:
+ case RCS:
+ mmio = RENDER_HWS_PGA_GEN7;
+ break;
+ case BCS:
+ mmio = BLT_HWS_PGA_GEN7;
+ break;
+ /*
+ * VCS2 actually doesn't exist on Gen7. Only shut up
+ * gcc switch check warning
+ */
+ case VCS2:
+ case VCS:
+ mmio = BSD_HWS_PGA_GEN7;
+ break;
+ case VECS:
+ mmio = VEBOX_HWS_PGA_GEN7;
+ break;
+ }
+ } else if (IS_GEN6(ring->dev)) {
+ mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
+ } else {
+ /* XXX: gen8 returns to sanity */
+ mmio = RING_HWS_PGA(ring->mmio_base);
}
- } else if (IS_GEN6(ring->dev)) {
- mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
- } else {
- /* XXX: gen8 returns to sanity */
- mmio = RING_HWS_PGA(ring->mmio_base);
}
- I915_WRITE(mmio, (u32)ring->status_page.gfx_addr);
+ I915_WRITE(mmio, addr);
POSTING_READ(mmio);
/*
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 06/15] drm/i915: Refactor the physical and virtual page hws setup
2014-08-05 14:51 ` [PATCH 06/15] drm/i915: Refactor the physical and virtual page hws setup Rodrigo Vivi
@ 2014-08-06 8:17 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:17 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:17AM -0700, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> We duplicated the legacy physical HWS setup routine for no good reason.
> Combine it with the more recent virtual HWS setup for simplicity.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
The duplication is with the legacy ums code, which (hopefully) will
disappear shortly. So no need for this patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_dma.c | 16 +------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 81 ++++++++++++++++-----------------
> 2 files changed, 39 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2e7f03a..f76d2bf 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -104,17 +104,6 @@ void i915_update_dri1_breadcrumb(struct drm_device *dev)
> }
> }
>
> -static void i915_write_hws_pga(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 addr;
> -
> - addr = dev_priv->status_page_dmah->busaddr;
> - if (INTEL_INFO(dev)->gen >= 4)
> - addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
> - I915_WRITE(HWS_PGA, addr);
> -}
> -
> /**
> * Frees the hardware status page, whether it's a physical address or a virtual
> * address set up by the X Server.
> @@ -255,10 +244,7 @@ static int i915_dma_resume(struct drm_device *dev)
> }
> DRM_DEBUG_DRIVER("hw status page @ %p\n",
> ring->status_page.page_addr);
> - if (ring->status_page.gfx_addr != 0)
> - intel_ring_setup_status_page(ring);
> - else
> - i915_write_hws_pga(dev);
> + intel_ring_setup_status_page(ring);
>
> DRM_DEBUG_DRIVER("Enabled hardware status page\n");
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b3d8f76..b7894d1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -443,17 +443,6 @@ u64 intel_ring_get_active_head(struct intel_engine_cs *ring)
> return acthd;
> }
>
> -static void ring_setup_phys_status_page(struct intel_engine_cs *ring)
> -{
> - struct drm_i915_private *dev_priv = ring->dev->dev_private;
> - u32 addr;
> -
> - addr = dev_priv->status_page_dmah->busaddr;
> - if (INTEL_INFO(ring->dev)->gen >= 4)
> - addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
> - I915_WRITE(HWS_PGA, addr);
> -}
> -
> static bool stop_ring(struct intel_engine_cs *ring)
> {
> struct drm_i915_private *dev_priv = to_i915(ring->dev);
> @@ -511,10 +500,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
> }
> }
>
> - if (I915_NEED_GFX_HWS(dev))
> - intel_ring_setup_status_page(ring);
> - else
> - ring_setup_phys_status_page(ring);
> + intel_ring_setup_status_page(ring);
>
> /* Initialize the ring. This must happen _after_ we've cleared the ring
> * registers with the above sequence (the readback of the HEAD registers
> @@ -1101,39 +1087,48 @@ void intel_ring_setup_status_page(struct intel_engine_cs *ring)
> {
> struct drm_device *dev = ring->dev;
> struct drm_i915_private *dev_priv = ring->dev->dev_private;
> - u32 mmio = 0;
> + u32 mmio, addr;
>
> - /* The ring status page addresses are no longer next to the rest of
> - * the ring registers as of gen7.
> - */
> - if (IS_GEN7(dev)) {
> - switch (ring->id) {
> - case RCS:
> - mmio = RENDER_HWS_PGA_GEN7;
> - break;
> - case BCS:
> - mmio = BLT_HWS_PGA_GEN7;
> - break;
> - /*
> - * VCS2 actually doesn't exist on Gen7. Only shut up
> - * gcc switch check warning
> + if (!I915_NEED_GFX_HWS(dev)) {
> + addr = dev_priv->status_page_dmah->busaddr;
> + if (INTEL_INFO(ring->dev)->gen >= 4)
> + addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
> + mmio = HWS_PGA;
> + } else {
> + addr = ring->status_page.gfx_addr;
> + /* The ring status page addresses are no longer next to the rest of
> + * the ring registers as of gen7.
> */
> - case VCS2:
> - case VCS:
> - mmio = BSD_HWS_PGA_GEN7;
> - break;
> - case VECS:
> - mmio = VEBOX_HWS_PGA_GEN7;
> - break;
> + if (IS_GEN7(dev)) {
> + switch (ring->id) {
> + default:
> + case RCS:
> + mmio = RENDER_HWS_PGA_GEN7;
> + break;
> + case BCS:
> + mmio = BLT_HWS_PGA_GEN7;
> + break;
> + /*
> + * VCS2 actually doesn't exist on Gen7. Only shut up
> + * gcc switch check warning
> + */
> + case VCS2:
> + case VCS:
> + mmio = BSD_HWS_PGA_GEN7;
> + break;
> + case VECS:
> + mmio = VEBOX_HWS_PGA_GEN7;
> + break;
> + }
> + } else if (IS_GEN6(ring->dev)) {
> + mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
> + } else {
> + /* XXX: gen8 returns to sanity */
> + mmio = RING_HWS_PGA(ring->mmio_base);
> }
> - } else if (IS_GEN6(ring->dev)) {
> - mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
> - } else {
> - /* XXX: gen8 returns to sanity */
> - mmio = RING_HWS_PGA(ring->mmio_base);
> }
>
> - I915_WRITE(mmio, (u32)ring->status_page.gfx_addr);
> + I915_WRITE(mmio, addr);
> POSTING_READ(mmio);
>
> /*
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 07/15] drm/i915: clean up PPGTT checking logic
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (5 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 06/15] drm/i915: Refactor the physical and virtual page hws setup Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:21 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 08/15] drm/i915: re-order ppgtt sanitize logic v2 Rodrigo Vivi
` (7 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Jesse Barnes <jbarnes@virtuousgeek.org>
sanitize_enable_ppgtt is the function that checks all the conditions,
honoring a forced ppgtt status or doing auto-detect as necessary. Just
make sure it returns the right value in all cases and use that in the
macros instead of the confusing intel_enable_ppgtt() function.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++-----------
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 -
3 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5de27f9..67e9da0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2048,8 +2048,8 @@ struct drm_i915_cmd_table {
#define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6)
#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))
-#define USES_PPGTT(dev) intel_enable_ppgtt(dev, false)
-#define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
+#define USES_PPGTT(dev) (i915.enable_ppgtt)
+#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt == 2)
#define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay)
#define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5188936..ffa4bb3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -33,17 +33,6 @@
static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv);
static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
-bool intel_enable_ppgtt(struct drm_device *dev, bool full)
-{
- if (i915.enable_ppgtt == 0)
- return false;
-
- if (i915.enable_ppgtt == 1 && full)
- return false;
-
- return true;
-}
-
static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
{
if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
@@ -70,6 +59,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
return 0;
}
+ if (HAS_PPGTT(dev))
+ return 2;
+
return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8d6f7c1..666c938 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -272,7 +272,6 @@ void i915_gem_init_global_gtt(struct drm_device *dev);
void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
unsigned long mappable_end, unsigned long end);
-bool intel_enable_ppgtt(struct drm_device *dev, bool full);
int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
void i915_check_and_clear_faults(struct drm_device *dev);
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 07/15] drm/i915: clean up PPGTT checking logic
2014-08-05 14:51 ` [PATCH 07/15] drm/i915: clean up PPGTT checking logic Rodrigo Vivi
@ 2014-08-06 8:21 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:21 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:18AM -0700, Rodrigo Vivi wrote:
> From: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> sanitize_enable_ppgtt is the function that checks all the conditions,
> honoring a forced ppgtt status or doing auto-detect as necessary. Just
> make sure it returns the right value in all cases and use that in the
> macros instead of the confusing intel_enable_ppgtt() function.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++-----------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 -
> 3 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5de27f9..67e9da0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2048,8 +2048,8 @@ struct drm_i915_cmd_table {
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))
> -#define USES_PPGTT(dev) intel_enable_ppgtt(dev, false)
> -#define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
> +#define USES_PPGTT(dev) (i915.enable_ppgtt)
> +#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt == 2)
>
> #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay)
> #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 5188936..ffa4bb3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -33,17 +33,6 @@
> static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv);
> static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
>
> -bool intel_enable_ppgtt(struct drm_device *dev, bool full)
> -{
> - if (i915.enable_ppgtt == 0)
> - return false;
> -
> - if (i915.enable_ppgtt == 1 && full)
> - return false;
> -
> - return true;
> -}
> -
> static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> {
> if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
> @@ -70,6 +59,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> return 0;
> }
>
> + if (HAS_PPGTT(dev))
> + return 2;
This reeneables ppgtt throught the backdoor. Dropped this hunk and merged
the patch.
-Daniel
> +
> return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8d6f7c1..666c938 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -272,7 +272,6 @@ void i915_gem_init_global_gtt(struct drm_device *dev);
> void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
> unsigned long mappable_end, unsigned long end);
>
> -bool intel_enable_ppgtt(struct drm_device *dev, bool full);
> int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
>
> void i915_check_and_clear_faults(struct drm_device *dev);
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 08/15] drm/i915: re-order ppgtt sanitize logic v2
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (6 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 07/15] drm/i915: clean up PPGTT checking logic Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:22 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 09/15] drm/i915: Bring GPU Freq to min while suspending Rodrigo Vivi
` (6 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Put hw limitations first, disabling ppgtt if necessary right away.
After that, check user passed args or auto-detect and do the right
thing, falling back to aliasing PPGTT if the user tries to enable full
PPGTT but it isn't available.
v2: simplify auto-detect case since we already caught the no PPGTT case early
on (Jesse)
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ffa4bb3..264f2a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -35,15 +35,6 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
{
- if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
- return 0;
-
- if (enable_ppgtt == 1)
- return 1;
-
- if (enable_ppgtt == 2 && HAS_PPGTT(dev))
- return 2;
-
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
@@ -59,10 +50,20 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
return 0;
}
- if (HAS_PPGTT(dev))
- return 2;
+ if (!HAS_ALIASING_PPGTT(dev))
+ return 0;
- return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
+ /* Check user passed enable_ppgtt param and try to honor it */
+ switch (enable_ppgtt) {
+ case 0:
+ return 0;
+ case 1:
+ return 1; /* caught any hw limits above */
+ case 2:
+ /* fall through to auto-detect */
+ default: /* auto-detect */
+ return HAS_PPGTT(dev) ? 2 : 1;
+ }
}
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 08/15] drm/i915: re-order ppgtt sanitize logic v2
2014-08-05 14:51 ` [PATCH 08/15] drm/i915: re-order ppgtt sanitize logic v2 Rodrigo Vivi
@ 2014-08-06 8:22 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:22 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:19AM -0700, Rodrigo Vivi wrote:
> From: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> Put hw limitations first, disabling ppgtt if necessary right away.
> After that, check user passed args or auto-detect and do the right
> thing, falling back to aliasing PPGTT if the user tries to enable full
> PPGTT but it isn't available.
>
> v2: simplify auto-detect case since we already caught the no PPGTT case early
> on (Jesse)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ffa4bb3..264f2a6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -35,15 +35,6 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
>
> static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> {
> - if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
> - return 0;
> -
> - if (enable_ppgtt == 1)
> - return 1;
> -
> - if (enable_ppgtt == 2 && HAS_PPGTT(dev))
> - return 2;
> -
> #ifdef CONFIG_INTEL_IOMMU
> /* Disable ppgtt on SNB if VT-d is on. */
> if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
> @@ -59,10 +50,20 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> return 0;
> }
>
> - if (HAS_PPGTT(dev))
> - return 2;
> + if (!HAS_ALIASING_PPGTT(dev))
> + return 0;
>
> - return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
> + /* Check user passed enable_ppgtt param and try to honor it */
> + switch (enable_ppgtt) {
> + case 0:
> + return 0;
> + case 1:
> + return 1; /* caught any hw limits above */
> + case 2:
> + /* fall through to auto-detect */
> + default: /* auto-detect */
> + return HAS_PPGTT(dev) ? 2 : 1;
Same issue, so not going to merge for now. Until that's address I guess we
can drop this.
-Daniel
> + }
> }
>
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 09/15] drm/i915: Bring GPU Freq to min while suspending.
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (7 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 08/15] drm/i915: re-order ppgtt sanitize logic v2 Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:23 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 10/15] drm/i915/bdw: Map unused PDPs to a scratch page Rodrigo Vivi
` (5 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Deepak S <deepak.s@linux.intel.com>
We might be leaving the PGU Frequency (and thus vnn) high during the suspend.
Flusing the delayed work queue should take care of this.
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5710571..ddfa279 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4559,7 +4559,7 @@ i915_gem_suspend(struct drm_device *dev)
del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
cancel_delayed_work_sync(&dev_priv->mm.retire_work);
- cancel_delayed_work_sync(&dev_priv->mm.idle_work);
+ flush_delayed_work(&dev_priv->mm.idle_work);
return 0;
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 09/15] drm/i915: Bring GPU Freq to min while suspending.
2014-08-05 14:51 ` [PATCH 09/15] drm/i915: Bring GPU Freq to min while suspending Rodrigo Vivi
@ 2014-08-06 8:23 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:23 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:20AM -0700, Rodrigo Vivi wrote:
> From: Deepak S <deepak.s@linux.intel.com>
>
> We might be leaving the PGU Frequency (and thus vnn) high during the suspend.
> Flusing the delayed work queue should take care of this.
>
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5710571..ddfa279 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4559,7 +4559,7 @@ i915_gem_suspend(struct drm_device *dev)
>
> del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
> cancel_delayed_work_sync(&dev_priv->mm.retire_work);
> - cancel_delayed_work_sync(&dev_priv->mm.idle_work);
> + flush_delayed_work(&dev_priv->mm.idle_work);
>
> return 0;
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 10/15] drm/i915/bdw: Map unused PDPs to a scratch page
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (8 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 09/15] drm/i915: Bring GPU Freq to min while suspending Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 11/15] drm/i915: Set M2_N2 registers during mode set Rodrigo Vivi
` (4 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Dave Gordon, Rodrigo Vivi
From: Bob Beckett <robert.beckett@intel.com>
Create a scratch page for the two unused PDPs and set all the PTEs
for them to point to it.
This patch addresses a page fault, and subsequent hang in pipe
control flush. In these cases, the Main Graphic Arbiter Error
register [0x40A0] showed a TLB Page Fault error, and a high memory
address (higher than the size of our PPGTT) was reported in the
Fault TLB RD Data0 register (0x4B10).
PDP2 & PDP3 were not set because, in theory, they aren't required
for our PPGTT size, but they should be mapped to a scratch page
anyway.
v2: Rebase on latest nightly.
Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
Signed-off-by: Dave Gordon <david.s.gordon@intel.com> (v2)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 79 +++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +
2 files changed, 65 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 264f2a6..a88f879 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -365,6 +365,11 @@ static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt)
kfree(ppgtt->gen8_pt_dma_addr[i]);
}
+ /* Unused PDPs are always assigned to scratch page */
+ for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++)
+ kfree(ppgtt->gen8_pt_dma_addr[i]);
+ __free_page(ppgtt->scratch_page);
+
__free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT));
}
@@ -389,6 +394,13 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
PCI_DMA_BIDIRECTIONAL);
}
}
+
+ /* Unused PDPs are always assigned to scratch page */
+ for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++) {
+ if (ppgtt->pd_dma_addr[i])
+ pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i],
+ PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ }
}
static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
@@ -475,10 +487,21 @@ static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
const int max_pdp)
{
- ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT));
- if (!ppgtt->pd_pages)
+ /* Scratch page for unmapped PDP's */
+ ppgtt->scratch_page = alloc_page(GFP_KERNEL);
+ if (!ppgtt->scratch_page)
return -ENOMEM;
+ /* Must allocate space for all 4 PDPs. HW has implemented cache which
+ * pre-fetches entries; that pre-fetch can attempt access for entries
+ * even if no resources are located in that range.
+ */
+ ppgtt->pd_pages = alloc_pages(GFP_KERNEL, GEN8_LEGACY_PDPS);
+ if (!ppgtt->pd_pages) {
+ __free_page(ppgtt->scratch_page);
+ return -ENOMEM;
+ }
+
ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT);
BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
@@ -496,6 +519,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp);
if (ret) {
+ __free_page(ppgtt->scratch_page);
__free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT));
return ret;
}
@@ -530,18 +554,25 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
const int pd,
- const int pt)
+ const int pt,
+ const int max_pdp)
{
dma_addr_t pt_addr;
struct page *p;
int ret;
- p = ppgtt->gen8_pt_pages[pd][pt];
- pt_addr = pci_map_page(ppgtt->base.dev->pdev,
- p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
- ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
- if (ret)
- return ret;
+ /* Unused PDPs need to have their ptes pointing to the
+ * existing scratch page.
+ */
+ if (pd < max_pdp) {
+ p = ppgtt->gen8_pt_pages[pd][pt];
+ pt_addr = pci_map_page(ppgtt->base.dev->pdev,
+ p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
+ if (ret)
+ return ret;
+ } else
+ pt_addr = ppgtt->scratch_dma_addr;
ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
@@ -563,6 +594,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
int i, j, ret;
+ gen8_gtt_pte_t *pt_vaddr, scratch_pte;
if (size % (1<<30))
DRM_INFO("Pages will be wasted unless GTT size (%llu) is divisible by 1GB\n", size);
@@ -572,30 +604,38 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
if (ret)
return ret;
- /*
- * 2. Create DMA mappings for the page directories and page tables.
- */
- for (i = 0; i < max_pdp; i++) {
+ /* 2. Map the scratch page */
+ ppgtt->scratch_dma_addr =
+ pci_map_page(ppgtt->base.dev->pdev,
+ ppgtt->scratch_page, 0, PAGE_SIZE,
+ PCI_DMA_BIDIRECTIONAL);
+
+ ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, ppgtt->scratch_dma_addr);
+ if (ret)
+ goto bail;
+
+ /* 3. Create DMA mappings for the page directories and page tables. */
+ for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
if (ret)
goto bail;
for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
- ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j);
+ ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j, max_pdp);
if (ret)
goto bail;
}
}
/*
- * 3. Map all the page directory entires to point to the page tables
+ * 4. Map all the page directory entries to point to the page tables
* we've allocated.
*
* For now, the PPGTT helper functions all require that the PDEs are
* plugged in correctly. So we do that now/here. For aliasing PPGTT, we
* will never need to touch the PDEs again.
*/
- for (i = 0; i < max_pdp; i++) {
+ for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
gen8_ppgtt_pde_t *pd_vaddr;
pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]);
for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
@@ -618,6 +658,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
+ scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr,
+ I915_CACHE_LLC, true);
+ pt_vaddr = kmap_atomic(ppgtt->scratch_page);
+ for (i = 0; i < GEN8_PTES_PER_PAGE; i++)
+ pt_vaddr[i] = scratch_pte;
+ kunmap_atomic(pt_vaddr);
+
DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d wasted)\n",
ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n",
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 666c938..02032b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -249,6 +249,7 @@ struct i915_hw_ppgtt {
struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
};
struct page *pd_pages;
+ struct page *scratch_page;
union {
uint32_t pd_offset;
dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
@@ -258,6 +259,7 @@ struct i915_hw_ppgtt {
dma_addr_t *gen8_pt_dma_addr[4];
};
+ dma_addr_t scratch_dma_addr;
struct intel_context *ctx;
int (*enable)(struct i915_hw_ppgtt *ppgtt);
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH 11/15] drm/i915: Set M2_N2 registers during mode set
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (9 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 10/15] drm/i915/bdw: Map unused PDPs to a scratch page Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-05 21:55 ` Jesse Barnes
2014-08-05 14:51 ` [PATCH 12/15] drm/i915: State readout and cross-checking for dp_m2_n2 Rodrigo Vivi
` (3 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi
From: Vandana Kannan <vandana.kannan@intel.com>
For Gen < 8, set M2_N2 registers on every mode set. This is required to make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
v2: Patch rebased
v3: Daniel's review comments
- Removed HAS_DRRS(dev) and added bool has_drrs to pipe_config to
track drrs support
v4: Jesse's review comments
- Made changes to set m2_n2 in intel_dp_set_m_n()
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_dp.c | 18 +++---------------
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
3 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 99eb7ca..e798acd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -91,11 +91,11 @@ static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj);
-static void intel_dp_set_m_n(struct intel_crtc *crtc);
static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
- struct intel_link_m_n *m_n);
+ struct intel_link_m_n *m_n,
+ struct intel_link_m_n *m2_n2);
static void ironlake_set_pipeconf(struct drm_crtc *crtc);
static void haswell_set_pipeconf(struct drm_crtc *crtc);
static void intel_set_pipe_csc(struct drm_crtc *crtc);
@@ -3980,7 +3980,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
- &intel_crtc->config.fdi_m_n);
+ &intel_crtc->config.fdi_m_n, NULL);
}
ironlake_set_pipeconf(crtc);
@@ -4093,7 +4093,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
- &intel_crtc->config.fdi_m_n);
+ &intel_crtc->config.fdi_m_n, NULL);
}
haswell_set_pipeconf(crtc);
@@ -5509,7 +5509,8 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
}
static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
- struct intel_link_m_n *m_n)
+ struct intel_link_m_n *m_n,
+ struct intel_link_m_n *m2_n2)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5521,6 +5522,18 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
+ /* M2_N2 registers to be set only for gen < 8 (M2_N2 available
+ * for gen < 8) and if DRRS is supported (to make sure the
+ * registers are not unnecessarily accessed).
+ */
+ if (m2_n2 && INTEL_INFO(dev)->gen < 8 &&
+ crtc->config.has_drrs) {
+ I915_WRITE(PIPE_DATA_M2(transcoder),
+ TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
+ I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
+ I915_WRITE(PIPE_LINK_M2(transcoder), m2_n2->link_m);
+ I915_WRITE(PIPE_LINK_N2(transcoder), m2_n2->link_n);
+ }
} else {
I915_WRITE(PIPE_DATA_M_G4X(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
I915_WRITE(PIPE_DATA_N_G4X(pipe), m_n->gmch_n);
@@ -5529,12 +5542,13 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
}
}
-static void intel_dp_set_m_n(struct intel_crtc *crtc)
+void intel_dp_set_m_n(struct intel_crtc *crtc)
{
if (crtc->config.has_pch_encoder)
intel_pch_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
else
- intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
+ intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n,
+ &crtc->config.dp_m2_n2);
}
static void vlv_update_pll(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ea6ff71..7290976 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -828,20 +828,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static void
-intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
-{
- struct drm_device *dev = crtc->base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- enum transcoder transcoder = crtc->config.cpu_transcoder;
-
- I915_WRITE(PIPE_DATA_M2(transcoder),
- TU_SIZE(m_n->tu) | m_n->gmch_m);
- I915_WRITE(PIPE_DATA_N2(transcoder), m_n->gmch_n);
- I915_WRITE(PIPE_LINK_M2(transcoder), m_n->link_m);
- I915_WRITE(PIPE_LINK_N2(transcoder), m_n->link_n);
-}
-
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config)
@@ -867,6 +853,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
pipe_config->has_pch_encoder = true;
pipe_config->has_dp_encoder = true;
+ pipe_config->has_drrs = false;
pipe_config->has_audio = intel_dp->has_audio;
if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
@@ -970,6 +957,7 @@ found:
if (intel_connector->panel.downclock_mode != NULL &&
intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) {
+ pipe_config->has_drrs = true;
intel_link_compute_m_n(bpp, lane_count,
intel_connector->panel.downclock_mode->clock,
pipe_config->port_clock,
@@ -4378,7 +4366,7 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
val = I915_READ(reg);
if (index > DRRS_HIGH_RR) {
val |= PIPECONF_EDP_RR_MODE_SWITCH;
- intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
+ intel_dp_set_m_n(intel_crtc);
} else {
val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fada887..1f675a8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -324,6 +324,7 @@ struct intel_crtc_config {
/* m2_n2 for eDP downclock */
struct intel_link_m_n dp_m2_n2;
+ bool has_drrs;
/*
* Frequence the dpll for the port should run at. Differs from the
@@ -876,6 +877,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv);
void hsw_disable_pc8(struct drm_i915_private *dev_priv);
void intel_dp_get_m_n(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config);
+void intel_dp_set_m_n(struct intel_crtc *crtc);
int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
void
ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
@@ -891,7 +893,6 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
int intel_format_to_fourcc(int format);
void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
-
/* intel_dp.c */
void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 11/15] drm/i915: Set M2_N2 registers during mode set
2014-08-05 14:51 ` [PATCH 11/15] drm/i915: Set M2_N2 registers during mode set Rodrigo Vivi
@ 2014-08-05 21:55 ` Jesse Barnes
0 siblings, 0 replies; 36+ messages in thread
From: Jesse Barnes @ 2014-08-05 21:55 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Daniel Vetter, intel-gfx
On Tue, 5 Aug 2014 07:51:22 -0700
Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> From: Vandana Kannan <vandana.kannan@intel.com>
>
> For Gen < 8, set M2_N2 registers on every mode set. This is required to make
> sure M2_N2 registers are set during boot, resume from sleep for cross-
> checking the state. The register is set only if DRRS is supported.
>
> v2: Patch rebased
>
> v3: Daniel's review comments
> - Removed HAS_DRRS(dev) and added bool has_drrs to pipe_config to
> track drrs support
>
> v4: Jesse's review comments
> - Made changes to set m2_n2 in intel_dp_set_m_n()
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_dp.c | 18 +++---------------
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> 3 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 99eb7ca..e798acd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -91,11 +91,11 @@ static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *ifb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_i915_gem_object *obj);
> -static void intel_dp_set_m_n(struct intel_crtc *crtc);
> static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
> static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
> static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> - struct intel_link_m_n *m_n);
> + struct intel_link_m_n *m_n,
> + struct intel_link_m_n *m2_n2);
> static void ironlake_set_pipeconf(struct drm_crtc *crtc);
> static void haswell_set_pipeconf(struct drm_crtc *crtc);
> static void intel_set_pipe_csc(struct drm_crtc *crtc);
> @@ -3980,7 +3980,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config.fdi_m_n);
> + &intel_crtc->config.fdi_m_n, NULL);
> }
>
> ironlake_set_pipeconf(crtc);
> @@ -4093,7 +4093,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config.fdi_m_n);
> + &intel_crtc->config.fdi_m_n, NULL);
> }
>
> haswell_set_pipeconf(crtc);
> @@ -5509,7 +5509,8 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
> }
>
> static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> - struct intel_link_m_n *m_n)
> + struct intel_link_m_n *m_n,
> + struct intel_link_m_n *m2_n2)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5521,6 +5522,18 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
> I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
> I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> + /* M2_N2 registers to be set only for gen < 8 (M2_N2 available
> + * for gen < 8) and if DRRS is supported (to make sure the
> + * registers are not unnecessarily accessed).
> + */
> + if (m2_n2 && INTEL_INFO(dev)->gen < 8 &&
> + crtc->config.has_drrs) {
> + I915_WRITE(PIPE_DATA_M2(transcoder),
> + TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
> + I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
> + I915_WRITE(PIPE_LINK_M2(transcoder), m2_n2->link_m);
> + I915_WRITE(PIPE_LINK_N2(transcoder), m2_n2->link_n);
> + }
> } else {
> I915_WRITE(PIPE_DATA_M_G4X(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> I915_WRITE(PIPE_DATA_N_G4X(pipe), m_n->gmch_n);
> @@ -5529,12 +5542,13 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> }
> }
>
> -static void intel_dp_set_m_n(struct intel_crtc *crtc)
> +void intel_dp_set_m_n(struct intel_crtc *crtc)
> {
> if (crtc->config.has_pch_encoder)
> intel_pch_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> else
> - intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
> + intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n,
> + &crtc->config.dp_m2_n2);
> }
>
> static void vlv_update_pll(struct intel_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ea6ff71..7290976 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -828,20 +828,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
> }
> }
>
> -static void
> -intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
> -{
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - enum transcoder transcoder = crtc->config.cpu_transcoder;
> -
> - I915_WRITE(PIPE_DATA_M2(transcoder),
> - TU_SIZE(m_n->tu) | m_n->gmch_m);
> - I915_WRITE(PIPE_DATA_N2(transcoder), m_n->gmch_n);
> - I915_WRITE(PIPE_LINK_M2(transcoder), m_n->link_m);
> - I915_WRITE(PIPE_LINK_N2(transcoder), m_n->link_n);
> -}
> -
> bool
> intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_config *pipe_config)
> @@ -867,6 +853,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> pipe_config->has_pch_encoder = true;
>
> pipe_config->has_dp_encoder = true;
> + pipe_config->has_drrs = false;
> pipe_config->has_audio = intel_dp->has_audio;
>
> if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> @@ -970,6 +957,7 @@ found:
>
> if (intel_connector->panel.downclock_mode != NULL &&
> intel_dp->drrs_state.type == SEAMLESS_DRRS_SUPPORT) {
> + pipe_config->has_drrs = true;
> intel_link_compute_m_n(bpp, lane_count,
> intel_connector->panel.downclock_mode->clock,
> pipe_config->port_clock,
> @@ -4378,7 +4366,7 @@ void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> val = I915_READ(reg);
> if (index > DRRS_HIGH_RR) {
> val |= PIPECONF_EDP_RR_MODE_SWITCH;
> - intel_dp_set_m2_n2(intel_crtc, &config->dp_m2_n2);
> + intel_dp_set_m_n(intel_crtc);
> } else {
> val &= ~PIPECONF_EDP_RR_MODE_SWITCH;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index fada887..1f675a8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -324,6 +324,7 @@ struct intel_crtc_config {
>
> /* m2_n2 for eDP downclock */
> struct intel_link_m_n dp_m2_n2;
> + bool has_drrs;
>
> /*
> * Frequence the dpll for the port should run at. Differs from the
> @@ -876,6 +877,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv);
> void hsw_disable_pc8(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config);
> +void intel_dp_set_m_n(struct intel_crtc *crtc);
> int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
> void
> ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> @@ -891,7 +893,6 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> int intel_format_to_fourcc(int format);
> void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
>
> -
> /* intel_dp.c */
> void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
There's an unnecessary gen8 check in there, but this one has my
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 12/15] drm/i915: State readout and cross-checking for dp_m2_n2
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (10 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 11/15] drm/i915: Set M2_N2 registers during mode set Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:25 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status Rodrigo Vivi
` (2 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi
From: Vandana Kannan <vandana.kannan@intel.com>
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.
v2: Changed patch title.
Daniel's review comments incorporated.
Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
only when high RR is not in use (This is because alternate m_n register
programming will be done only when low RR is being used).
v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
based on DRRS state for gen 8 and above.
Save and restore M2 N2 registers for gen 7 and below
v4: For Gen>=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
only one set of M_N registers
v5: Removed the chunk which saves and restores M2_N2 registers. Modified
get_m_n() to get M2_N2 registers as well. Modified the macro which compares
hw.dp_m_n against sw.dp_m2_n2/sw.dp_m_n for gen > 8.
v6: Added check to compare dp_m2_n2 only when DRRS is enabled
v7: Modified drrs check to use has_drrs
v8: Add has_drrs check before reading M2_N2 registers
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++++++++----
1 file changed, 67 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e798acd..b758826 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7145,7 +7145,8 @@ static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
enum transcoder transcoder,
- struct intel_link_m_n *m_n)
+ struct intel_link_m_n *m_n,
+ struct intel_link_m_n *m2_n2)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7159,6 +7160,20 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
& TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+ /* Read M2_N2 registers only for gen < 8 (M2_N2 available for
+ * gen < 8) and if DRRS is supported (to make sure the
+ * registers are not unnecessarily read).
+ */
+ if (m2_n2 && INTEL_INFO(dev)->gen < 8 &&
+ crtc->config.has_drrs) {
+ m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
+ m2_n2->link_n = I915_READ(PIPE_LINK_N2(transcoder));
+ m2_n2->gmch_m = I915_READ(PIPE_DATA_M2(transcoder))
+ & ~TU_SIZE_MASK;
+ m2_n2->gmch_n = I915_READ(PIPE_DATA_N2(transcoder));
+ m2_n2->tu = ((I915_READ(PIPE_DATA_M2(transcoder))
+ & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+ }
} else {
m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
@@ -7177,14 +7192,15 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n);
else
intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
- &pipe_config->dp_m_n);
+ &pipe_config->dp_m_n,
+ &pipe_config->dp_m2_n2);
}
static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
- &pipe_config->fdi_m_n);
+ &pipe_config->fdi_m_n, NULL);
}
static void ironlake_get_pfit_config(struct intel_crtc *crtc,
@@ -10001,6 +10017,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
pipe_config->dp_m_n.tu);
+
+ DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: %u, tu2: %u\n",
+ pipe_config->has_dp_encoder,
+ pipe_config->dp_m2_n2.gmch_m,
+ pipe_config->dp_m2_n2.gmch_n,
+ pipe_config->dp_m2_n2.link_m,
+ pipe_config->dp_m2_n2.link_n,
+ pipe_config->dp_m2_n2.tu);
+
DRM_DEBUG_KMS("requested mode:\n");
drm_mode_debug_printmodeline(&pipe_config->requested_mode);
DRM_DEBUG_KMS("adjusted mode:\n");
@@ -10381,6 +10406,22 @@ intel_pipe_config_compare(struct drm_device *dev,
return false; \
}
+/* This is required for BDW+ where there is only one set of registers for
+ * switching between high and low RR.
+ * This macro can be used whenever a comparison has to be made between one
+ * hw state and multiple sw state variables.
+ */
+#define PIPE_CONF_CHECK_I_ALT(name, alt_name) \
+ if ((current_config->name != pipe_config->name) && \
+ (current_config->alt_name != pipe_config->name)) { \
+ DRM_ERROR("mismatch in " #name " " \
+ "(expected %i or %i, found %i)\n", \
+ current_config->name, \
+ current_config->alt_name, \
+ pipe_config->name); \
+ return false; \
+ }
+
#define PIPE_CONF_CHECK_FLAGS(name, mask) \
if ((current_config->name ^ pipe_config->name) & (mask)) { \
DRM_ERROR("mismatch in " #name "(" #mask ") " \
@@ -10413,11 +10454,28 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_I(fdi_m_n.tu);
PIPE_CONF_CHECK_I(has_dp_encoder);
- PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
- PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
- PIPE_CONF_CHECK_I(dp_m_n.link_m);
- PIPE_CONF_CHECK_I(dp_m_n.link_n);
- PIPE_CONF_CHECK_I(dp_m_n.tu);
+
+ if (INTEL_INFO(dev)->gen < 8) {
+ PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
+ PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
+ PIPE_CONF_CHECK_I(dp_m_n.link_m);
+ PIPE_CONF_CHECK_I(dp_m_n.link_n);
+ PIPE_CONF_CHECK_I(dp_m_n.tu);
+
+ if (current_config->has_drrs) {
+ PIPE_CONF_CHECK_I(dp_m2_n2.gmch_m);
+ PIPE_CONF_CHECK_I(dp_m2_n2.gmch_n);
+ PIPE_CONF_CHECK_I(dp_m2_n2.link_m);
+ PIPE_CONF_CHECK_I(dp_m2_n2.link_n);
+ PIPE_CONF_CHECK_I(dp_m2_n2.tu);
+ }
+ } else {
+ PIPE_CONF_CHECK_I_ALT(dp_m_n.gmch_m, dp_m2_n2.gmch_m);
+ PIPE_CONF_CHECK_I_ALT(dp_m_n.gmch_n, dp_m2_n2.gmch_n);
+ PIPE_CONF_CHECK_I_ALT(dp_m_n.link_m, dp_m2_n2.link_m);
+ PIPE_CONF_CHECK_I_ALT(dp_m_n.link_n, dp_m2_n2.link_n);
+ PIPE_CONF_CHECK_I_ALT(dp_m_n.tu, dp_m2_n2.tu);
+ }
PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
@@ -10503,6 +10561,7 @@ intel_pipe_config_compare(struct drm_device *dev,
#undef PIPE_CONF_CHECK_X
#undef PIPE_CONF_CHECK_I
+#undef PIPE_CONF_CHECK_I_ALT
#undef PIPE_CONF_CHECK_FLAGS
#undef PIPE_CONF_CHECK_CLOCK_FUZZY
#undef PIPE_CONF_QUIRK
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 12/15] drm/i915: State readout and cross-checking for dp_m2_n2
2014-08-05 14:51 ` [PATCH 12/15] drm/i915: State readout and cross-checking for dp_m2_n2 Rodrigo Vivi
@ 2014-08-06 8:25 ` Daniel Vetter
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:25 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Daniel Vetter, intel-gfx
On Tue, Aug 05, 2014 at 07:51:23AM -0700, Rodrigo Vivi wrote:
> From: Vandana Kannan <vandana.kannan@intel.com>
>
> Adding relevant read out comparison code, in check_crtc_state, for the new
> member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
> values for a DP downclock mode (if available). Suggested by Daniel.
>
> v2: Changed patch title.
> Daniel's review comments incorporated.
> Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
> only when high RR is not in use (This is because alternate m_n register
> programming will be done only when low RR is being used).
>
> v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
> Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
> based on DRRS state for gen 8 and above.
> Save and restore M2 N2 registers for gen 7 and below
>
> v4: For Gen>=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
> only one set of M_N registers
>
> v5: Removed the chunk which saves and restores M2_N2 registers. Modified
> get_m_n() to get M2_N2 registers as well. Modified the macro which compares
> hw.dp_m_n against sw.dp_m2_n2/sw.dp_m_n for gen > 8.
>
> v6: Added check to compare dp_m2_n2 only when DRRS is enabled
>
> v7: Modified drrs check to use has_drrs
>
> v8: Add has_drrs check before reading M2_N2 registers
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Both drrs patches pulled in, thanks.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++++++++----
> 1 file changed, 67 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e798acd..b758826 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7145,7 +7145,8 @@ static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
>
> static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
> enum transcoder transcoder,
> - struct intel_link_m_n *m_n)
> + struct intel_link_m_n *m_n,
> + struct intel_link_m_n *m2_n2)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7159,6 +7160,20 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
> m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
> m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
> & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> + /* Read M2_N2 registers only for gen < 8 (M2_N2 available for
> + * gen < 8) and if DRRS is supported (to make sure the
> + * registers are not unnecessarily read).
> + */
> + if (m2_n2 && INTEL_INFO(dev)->gen < 8 &&
> + crtc->config.has_drrs) {
> + m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
> + m2_n2->link_n = I915_READ(PIPE_LINK_N2(transcoder));
> + m2_n2->gmch_m = I915_READ(PIPE_DATA_M2(transcoder))
> + & ~TU_SIZE_MASK;
> + m2_n2->gmch_n = I915_READ(PIPE_DATA_N2(transcoder));
> + m2_n2->tu = ((I915_READ(PIPE_DATA_M2(transcoder))
> + & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> + }
> } else {
> m_n->link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
> m_n->link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
> @@ -7177,14 +7192,15 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
> intel_pch_transcoder_get_m_n(crtc, &pipe_config->dp_m_n);
> else
> intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> - &pipe_config->dp_m_n);
> + &pipe_config->dp_m_n,
> + &pipe_config->dp_m2_n2);
> }
>
> static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config)
> {
> intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder,
> - &pipe_config->fdi_m_n);
> + &pipe_config->fdi_m_n, NULL);
> }
>
> static void ironlake_get_pfit_config(struct intel_crtc *crtc,
> @@ -10001,6 +10017,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
> pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
> pipe_config->dp_m_n.tu);
> +
> + DRM_DEBUG_KMS("dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: %u, tu2: %u\n",
> + pipe_config->has_dp_encoder,
> + pipe_config->dp_m2_n2.gmch_m,
> + pipe_config->dp_m2_n2.gmch_n,
> + pipe_config->dp_m2_n2.link_m,
> + pipe_config->dp_m2_n2.link_n,
> + pipe_config->dp_m2_n2.tu);
> +
> DRM_DEBUG_KMS("requested mode:\n");
> drm_mode_debug_printmodeline(&pipe_config->requested_mode);
> DRM_DEBUG_KMS("adjusted mode:\n");
> @@ -10381,6 +10406,22 @@ intel_pipe_config_compare(struct drm_device *dev,
> return false; \
> }
>
> +/* This is required for BDW+ where there is only one set of registers for
> + * switching between high and low RR.
> + * This macro can be used whenever a comparison has to be made between one
> + * hw state and multiple sw state variables.
> + */
> +#define PIPE_CONF_CHECK_I_ALT(name, alt_name) \
> + if ((current_config->name != pipe_config->name) && \
> + (current_config->alt_name != pipe_config->name)) { \
> + DRM_ERROR("mismatch in " #name " " \
> + "(expected %i or %i, found %i)\n", \
> + current_config->name, \
> + current_config->alt_name, \
> + pipe_config->name); \
> + return false; \
> + }
> +
> #define PIPE_CONF_CHECK_FLAGS(name, mask) \
> if ((current_config->name ^ pipe_config->name) & (mask)) { \
> DRM_ERROR("mismatch in " #name "(" #mask ") " \
> @@ -10413,11 +10454,28 @@ intel_pipe_config_compare(struct drm_device *dev,
> PIPE_CONF_CHECK_I(fdi_m_n.tu);
>
> PIPE_CONF_CHECK_I(has_dp_encoder);
> - PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
> - PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
> - PIPE_CONF_CHECK_I(dp_m_n.link_m);
> - PIPE_CONF_CHECK_I(dp_m_n.link_n);
> - PIPE_CONF_CHECK_I(dp_m_n.tu);
> +
> + if (INTEL_INFO(dev)->gen < 8) {
> + PIPE_CONF_CHECK_I(dp_m_n.gmch_m);
> + PIPE_CONF_CHECK_I(dp_m_n.gmch_n);
> + PIPE_CONF_CHECK_I(dp_m_n.link_m);
> + PIPE_CONF_CHECK_I(dp_m_n.link_n);
> + PIPE_CONF_CHECK_I(dp_m_n.tu);
> +
> + if (current_config->has_drrs) {
> + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_m);
> + PIPE_CONF_CHECK_I(dp_m2_n2.gmch_n);
> + PIPE_CONF_CHECK_I(dp_m2_n2.link_m);
> + PIPE_CONF_CHECK_I(dp_m2_n2.link_n);
> + PIPE_CONF_CHECK_I(dp_m2_n2.tu);
> + }
> + } else {
> + PIPE_CONF_CHECK_I_ALT(dp_m_n.gmch_m, dp_m2_n2.gmch_m);
> + PIPE_CONF_CHECK_I_ALT(dp_m_n.gmch_n, dp_m2_n2.gmch_n);
> + PIPE_CONF_CHECK_I_ALT(dp_m_n.link_m, dp_m2_n2.link_m);
> + PIPE_CONF_CHECK_I_ALT(dp_m_n.link_n, dp_m2_n2.link_n);
> + PIPE_CONF_CHECK_I_ALT(dp_m_n.tu, dp_m2_n2.tu);
> + }
>
> PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
> PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
> @@ -10503,6 +10561,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>
> #undef PIPE_CONF_CHECK_X
> #undef PIPE_CONF_CHECK_I
> +#undef PIPE_CONF_CHECK_I_ALT
> #undef PIPE_CONF_CHECK_FLAGS
> #undef PIPE_CONF_CHECK_CLOCK_FUZZY
> #undef PIPE_CONF_QUIRK
> --
> 1.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (11 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 12/15] drm/i915: State readout and cross-checking for dp_m2_n2 Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 3:32 ` Sharma, Shashank
2014-08-06 6:44 ` Dave Airlie
2014-08-05 14:51 ` [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8 Rodrigo Vivi
2014-08-05 14:51 ` [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv Rodrigo Vivi
14 siblings, 2 replies; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Ramalingam C <ramalingam.c@intel.com>
This change uses the HPD pins live status bit from
South Display Engine(SDE) to identify the HDMI hotplug state.
On Soft HPD events (on automated test cases) only HPD pin will be
toggled to notify the HDMI state change. But physical DDC will
be alive. Similarly on slow HDMI hotplug out, because of the physical
interface design, DDC remains active for short duration even when
HPD live status is indicating the disconnect state. Because of this
on VLV and HSW, slow hotplug out events and soft HPDs are not captured.
Hence this patch uses the HPD pins live status to identify the
HDMI connector status and allows EDID retrival only when live status
is up.
Change-Id: I958b57fa139e52b45c8b349c861cb8eab7b67ae5
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 10 +++++
drivers/gpu/drm/i915/intel_hdmi.c | 87 +++++++++++++++++++++++++++++++++------
2 files changed, 85 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1f675a8..b2e2e0c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -500,6 +500,16 @@ struct cxsr_latency {
#define to_intel_plane(x) container_of(x, struct intel_plane, base)
#define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
+/* DisplayPort/HDMI Hotplug line status bit mask */
+#define VLV_HDMIB_HOTPLUG_LIVE_STATUS (1 << 29)
+#define VLV_HDMIC_HOTPLUG_LIVE_STATUS (1 << 28)
+#define VLV_HDMID_HOTPLUG_LIVE_STATUS (1 << 27)
+
+/* DisplayPort/HDMI/DVI Hotplug line status bit mask */
+#define CORE_HDMIB_HOTPLUG_LIVE_STATUS (1 << 21)
+#define CORE_HDMIC_HOTPLUG_LIVE_STATUS (1 << 22)
+#define CORE_HDMID_HOTPLUG_LIVE_STATUS (1 << 23)
+
struct intel_hdmi {
u32 hdmi_reg;
int ddc_bus;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f9151f6..1920e4e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -962,6 +962,61 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
return true;
}
+static int get_hdmi_hotplug_live_status(struct drm_device *dev,
+ struct intel_hdmi *intel_hdmi)
+{
+ uint32_t bit, reg;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_digital_port *intel_dig_port =
+ hdmi_to_dig_port(intel_hdmi);
+
+ DRM_DEBUG_KMS("Reading Live status");
+
+ /* Live status is available from Gen 6 onwards */
+ if (INTEL_INFO(dev)->gen < 6)
+ return connector_status_connected;
+
+ if (IS_VALLEYVIEW(dev)) {
+ switch (intel_dig_port->port) {
+ case PORT_B:
+ bit = VLV_HDMIB_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_C:
+ bit = VLV_HDMIC_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_D:
+ bit = VLV_HDMID_HOTPLUG_LIVE_STATUS;
+ break;
+ default:
+ DRM_ERROR("Unrecognized port is encountered\n");
+ return connector_status_unknown;
+ }
+ reg = I915_READ(PORT_HOTPLUG_STAT);
+
+ } else {
+ switch (intel_dig_port->port) {
+ case PORT_B:
+ bit = CORE_HDMIB_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_C:
+ bit = CORE_HDMIC_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_D:
+ bit = CORE_HDMID_HOTPLUG_LIVE_STATUS;
+ break;
+ default:
+ DRM_ERROR("Unrecognized port is encountered\n");
+ return connector_status_unknown;
+ }
+
+ reg = I915_READ(SDEISR);
+ }
+
+ /* Return connector status */
+ return ((reg & bit) ?
+ connector_status_connected : connector_status_disconnected);
+}
+
static enum drm_connector_status
intel_hdmi_detect(struct drm_connector *connector, bool force)
{
@@ -981,24 +1036,32 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_get(dev_priv, power_domain);
+ status = get_hdmi_hotplug_live_status(dev, intel_hdmi);
+
intel_hdmi->has_hdmi_sink = false;
intel_hdmi->has_audio = false;
intel_hdmi->rgb_quant_range_selectable = false;
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv,
- intel_hdmi->ddc_bus));
- if (edid) {
- if (edid->input & DRM_EDID_INPUT_DIGITAL) {
- status = connector_status_connected;
- if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
- intel_hdmi->has_hdmi_sink =
+ if (status == connector_status_connected) {
+ edid = drm_get_edid(connector,
+ intel_gmbus_get_adapter(dev_priv,
+ intel_hdmi->ddc_bus));
+ if (edid) {
+ if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+ status = connector_status_connected;
+ if (intel_hdmi->force_audio !=
+ HDMI_AUDIO_OFF_DVI)
+ intel_hdmi->has_hdmi_sink =
drm_detect_hdmi_monitor(edid);
- intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->rgb_quant_range_selectable =
- drm_rgb_quant_range_selectable(edid);
+ intel_hdmi->has_audio =
+ drm_detect_monitor_audio(edid);
+ intel_hdmi->rgb_quant_range_selectable =
+ drm_rgb_quant_range_selectable(edid);
+ }
+ kfree(edid);
+ } else {
+ status = connector_status_disconnected;
}
- kfree(edid);
}
if (status == connector_status_connected) {
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status
2014-08-05 14:51 ` [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status Rodrigo Vivi
@ 2014-08-06 3:32 ` Sharma, Shashank
2014-08-06 6:44 ` Dave Airlie
1 sibling, 0 replies; 36+ messages in thread
From: Sharma, Shashank @ 2014-08-06 3:32 UTC (permalink / raw)
To: Vivi, Rodrigo, intel-gfx
We have prepared a patch set for HDMI design change, which will not break old platforms, cache the EDID for a finite duration, support HDMI compliance etc.
Please wait for a day or two, while we are doing the ULT. That patch is a superset of this, and will cover live_status based detection also.
Regards
Shashank
-----Original Message-----
From: Vivi, Rodrigo
Sent: Tuesday, August 05, 2014 8:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: C, Ramalingam; Sharma, Shashank; Vivi, Rodrigo
Subject: [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status
From: Ramalingam C <ramalingam.c@intel.com>
This change uses the HPD pins live status bit from South Display Engine(SDE) to identify the HDMI hotplug state.
On Soft HPD events (on automated test cases) only HPD pin will be toggled to notify the HDMI state change. But physical DDC will be alive. Similarly on slow HDMI hotplug out, because of the physical interface design, DDC remains active for short duration even when HPD live status is indicating the disconnect state. Because of this on VLV and HSW, slow hotplug out events and soft HPDs are not captured.
Hence this patch uses the HPD pins live status to identify the HDMI connector status and allows EDID retrival only when live status is up.
Change-Id: I958b57fa139e52b45c8b349c861cb8eab7b67ae5
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 10 +++++ drivers/gpu/drm/i915/intel_hdmi.c | 87 +++++++++++++++++++++++++++++++++------
2 files changed, 85 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1f675a8..b2e2e0c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -500,6 +500,16 @@ struct cxsr_latency { #define to_intel_plane(x) container_of(x, struct intel_plane, base) #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
+/* DisplayPort/HDMI Hotplug line status bit mask */
+#define VLV_HDMIB_HOTPLUG_LIVE_STATUS (1 << 29)
+#define VLV_HDMIC_HOTPLUG_LIVE_STATUS (1 << 28)
+#define VLV_HDMID_HOTPLUG_LIVE_STATUS (1 << 27)
+
+/* DisplayPort/HDMI/DVI Hotplug line status bit mask */ #define
+CORE_HDMIB_HOTPLUG_LIVE_STATUS (1 << 21) #define
+CORE_HDMIC_HOTPLUG_LIVE_STATUS (1 << 22) #define
+CORE_HDMID_HOTPLUG_LIVE_STATUS (1 << 23)
+
struct intel_hdmi {
u32 hdmi_reg;
int ddc_bus;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f9151f6..1920e4e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -962,6 +962,61 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
return true;
}
+static int get_hdmi_hotplug_live_status(struct drm_device *dev,
+ struct intel_hdmi *intel_hdmi)
+{
+ uint32_t bit, reg;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_digital_port *intel_dig_port =
+ hdmi_to_dig_port(intel_hdmi);
+
+ DRM_DEBUG_KMS("Reading Live status");
+
+ /* Live status is available from Gen 6 onwards */
+ if (INTEL_INFO(dev)->gen < 6)
+ return connector_status_connected;
+
+ if (IS_VALLEYVIEW(dev)) {
+ switch (intel_dig_port->port) {
+ case PORT_B:
+ bit = VLV_HDMIB_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_C:
+ bit = VLV_HDMIC_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_D:
+ bit = VLV_HDMID_HOTPLUG_LIVE_STATUS;
+ break;
+ default:
+ DRM_ERROR("Unrecognized port is encountered\n");
+ return connector_status_unknown;
+ }
+ reg = I915_READ(PORT_HOTPLUG_STAT);
+
+ } else {
+ switch (intel_dig_port->port) {
+ case PORT_B:
+ bit = CORE_HDMIB_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_C:
+ bit = CORE_HDMIC_HOTPLUG_LIVE_STATUS;
+ break;
+ case PORT_D:
+ bit = CORE_HDMID_HOTPLUG_LIVE_STATUS;
+ break;
+ default:
+ DRM_ERROR("Unrecognized port is encountered\n");
+ return connector_status_unknown;
+ }
+
+ reg = I915_READ(SDEISR);
+ }
+
+ /* Return connector status */
+ return ((reg & bit) ?
+ connector_status_connected : connector_status_disconnected); }
+
static enum drm_connector_status
intel_hdmi_detect(struct drm_connector *connector, bool force) { @@ -981,24 +1036,32 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_get(dev_priv, power_domain);
+ status = get_hdmi_hotplug_live_status(dev, intel_hdmi);
+
intel_hdmi->has_hdmi_sink = false;
intel_hdmi->has_audio = false;
intel_hdmi->rgb_quant_range_selectable = false;
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv,
- intel_hdmi->ddc_bus));
- if (edid) {
- if (edid->input & DRM_EDID_INPUT_DIGITAL) {
- status = connector_status_connected;
- if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
- intel_hdmi->has_hdmi_sink =
+ if (status == connector_status_connected) {
+ edid = drm_get_edid(connector,
+ intel_gmbus_get_adapter(dev_priv,
+ intel_hdmi->ddc_bus));
+ if (edid) {
+ if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+ status = connector_status_connected;
+ if (intel_hdmi->force_audio !=
+ HDMI_AUDIO_OFF_DVI)
+ intel_hdmi->has_hdmi_sink =
drm_detect_hdmi_monitor(edid);
- intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->rgb_quant_range_selectable =
- drm_rgb_quant_range_selectable(edid);
+ intel_hdmi->has_audio =
+ drm_detect_monitor_audio(edid);
+ intel_hdmi->rgb_quant_range_selectable =
+ drm_rgb_quant_range_selectable(edid);
+ }
+ kfree(edid);
+ } else {
+ status = connector_status_disconnected;
}
- kfree(edid);
}
if (status == connector_status_connected) {
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status
2014-08-05 14:51 ` [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status Rodrigo Vivi
2014-08-06 3:32 ` Sharma, Shashank
@ 2014-08-06 6:44 ` Dave Airlie
1 sibling, 0 replies; 36+ messages in thread
From: Dave Airlie @ 2014-08-06 6:44 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On 6 August 2014 00:51, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> From: Ramalingam C <ramalingam.c@intel.com>
>
> This change uses the HPD pins live status bit from
> South Display Engine(SDE) to identify the HDMI hotplug state.
ibx_digital_port_connected?
I think a lot of this patch exists already.
Dave.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (12 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 13/15] drm/i915: HDMI detection based on HPD pin live status Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 8:27 ` Daniel Vetter
2014-08-05 14:51 ` [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv Rodrigo Vivi
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Michel Thierry <michel.thierry@intel.com>
After unclaimed register detection was enabled for BDW, I started seeing
warnings while reading registers 0x4400c (DEIER) and 0x4401c (GTIER).
>From Gen8, DEIER has been split per display engine pipe, and GTIER has
been split in 4.
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 67e9da0..6622a53 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -314,6 +314,8 @@ struct drm_i915_error_state {
u32 eir;
u32 pgtbl_er;
u32 ier;
+ u32 pipe_ier[I915_MAX_PIPES]; /* gen8 */
+ u32 gt_ier[4]; /* gen8 */
u32 ccid;
u32 derrmr;
u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f694..e7a4ae0 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -380,6 +380,16 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
if (INTEL_INFO(dev)->gen == 7)
err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
+ if (INTEL_INFO(dev)->gen == 8) {
+ for (i = 0; i < ARRAY_SIZE(error->pipe_ier); i++)
+ err_printf(m, "PIPE_IER_%d: 0x%08x\n", i,
+ error->pipe_ier[i]);
+
+ for (i = 0; i < ARRAY_SIZE(error->gt_ier); i++)
+ err_printf(m, "GT_IER_%d: 0x%08x\n", i,
+ error->gt_ier[i]);
+ }
+
for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
err_printf(m, "%s command stream:\n", ring_str(i));
i915_ring_error_state(m, dev, &error->ring[i]);
@@ -1091,6 +1101,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
struct drm_i915_error_state *error)
{
struct drm_device *dev = dev_priv->dev;
+ int i, pipe;
/* General organization
* 1. Registers specific to a single generation
@@ -1135,7 +1146,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
if (HAS_HW_CONTEXTS(dev))
error->ccid = I915_READ(CCID);
- if (HAS_PCH_SPLIT(dev))
+ if (IS_GEN8(dev)) {
+ for_each_pipe(pipe)
+ error->pipe_ier[pipe] =
+ I915_READ(GEN8_DE_PIPE_IER(pipe));
+ for (i = 0; i < 4; i++)
+ error->gt_ier[i] = I915_READ(GEN8_GT_IER(i));
+ } else if (HAS_PCH_SPLIT(dev))
error->ier = I915_READ(DEIER) | I915_READ(GTIER);
else {
if (IS_GEN2(dev))
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8
2014-08-05 14:51 ` [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8 Rodrigo Vivi
@ 2014-08-06 8:27 ` Daniel Vetter
2014-08-06 8:56 ` Thierry, Michel
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Vetter @ 2014-08-06 8:27 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:25AM -0700, Rodrigo Vivi wrote:
> From: Michel Thierry <michel.thierry@intel.com>
>
> After unclaimed register detection was enabled for BDW, I started seeing
> warnings while reading registers 0x4400c (DEIER) and 0x4401c (GTIER).
>
> From Gen8, DEIER has been split per display engine pipe, and GTIER has
> been split in 4.
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Isn't that now addressed by the other gt/deier patches from you?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 67e9da0..6622a53 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -314,6 +314,8 @@ struct drm_i915_error_state {
> u32 eir;
> u32 pgtbl_er;
> u32 ier;
> + u32 pipe_ier[I915_MAX_PIPES]; /* gen8 */
> + u32 gt_ier[4]; /* gen8 */
> u32 ccid;
> u32 derrmr;
> u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 0b3f694..e7a4ae0 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -380,6 +380,16 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> if (INTEL_INFO(dev)->gen == 7)
> err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
>
> + if (INTEL_INFO(dev)->gen == 8) {
> + for (i = 0; i < ARRAY_SIZE(error->pipe_ier); i++)
> + err_printf(m, "PIPE_IER_%d: 0x%08x\n", i,
> + error->pipe_ier[i]);
> +
> + for (i = 0; i < ARRAY_SIZE(error->gt_ier); i++)
> + err_printf(m, "GT_IER_%d: 0x%08x\n", i,
> + error->gt_ier[i]);
> + }
> +
> for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
> err_printf(m, "%s command stream:\n", ring_str(i));
> i915_ring_error_state(m, dev, &error->ring[i]);
> @@ -1091,6 +1101,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> struct drm_i915_error_state *error)
> {
> struct drm_device *dev = dev_priv->dev;
> + int i, pipe;
>
> /* General organization
> * 1. Registers specific to a single generation
> @@ -1135,7 +1146,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> if (HAS_HW_CONTEXTS(dev))
> error->ccid = I915_READ(CCID);
>
> - if (HAS_PCH_SPLIT(dev))
> + if (IS_GEN8(dev)) {
> + for_each_pipe(pipe)
> + error->pipe_ier[pipe] =
> + I915_READ(GEN8_DE_PIPE_IER(pipe));
> + for (i = 0; i < 4; i++)
> + error->gt_ier[i] = I915_READ(GEN8_GT_IER(i));
> + } else if (HAS_PCH_SPLIT(dev))
> error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> else {
> if (IS_GEN2(dev))
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8
2014-08-06 8:27 ` Daniel Vetter
@ 2014-08-06 8:56 ` Thierry, Michel
0 siblings, 0 replies; 36+ messages in thread
From: Thierry, Michel @ 2014-08-06 8:56 UTC (permalink / raw)
To: Daniel Vetter, Vivi, Rodrigo; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 3891 bytes --]
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Daniel Vetter
> Sent: Wednesday, August 06, 2014 9:28 AM
> To: Vivi, Rodrigo
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: capture_reg_state
interrupt
> registers for Gen8
>
> On Tue, Aug 05, 2014 at 07:51:25AM -0700, Rodrigo Vivi wrote:
> > From: Michel Thierry <michel.thierry@intel.com>
> >
> > After unclaimed register detection was enabled for BDW, I started seeing
> > warnings while reading registers 0x4400c (DEIER) and 0x4401c (GTIER).
> >
> > From Gen8, DEIER has been split per display engine pipe, and GTIER has
> > been split in 4.
> >
> > Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Isn't that now addressed by the other gt/deier patches from you?
> -Daniel
Yes, Rodrigo's patch (drm/i915: Fix DEIER and GTIER collecting for BDW)
addressed the same thing.
--Michel
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++++++-
> > 2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index 67e9da0..6622a53 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -314,6 +314,8 @@ struct drm_i915_error_state {
> > u32 eir;
> > u32 pgtbl_er;
> > u32 ier;
> > + u32 pipe_ier[I915_MAX_PIPES]; /* gen8 */
> > + u32 gt_ier[4]; /* gen8 */
> > u32 ccid;
> > u32 derrmr;
> > u32 forcewake;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 0b3f694..e7a4ae0 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -380,6 +380,16 @@ int i915_error_state_to_str(struct
> drm_i915_error_state_buf *m,
> > if (INTEL_INFO(dev)->gen == 7)
> > err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
> >
> > + if (INTEL_INFO(dev)->gen == 8) {
> > + for (i = 0; i < ARRAY_SIZE(error->pipe_ier); i++)
> > + err_printf(m, "PIPE_IER_%d: 0x%08x\n", i,
> > + error->pipe_ier[i]);
> > +
> > + for (i = 0; i < ARRAY_SIZE(error->gt_ier); i++)
> > + err_printf(m, "GT_IER_%d: 0x%08x\n", i,
> > + error->gt_ier[i]);
> > + }
> > +
> > for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
> > err_printf(m, "%s command stream:\n", ring_str(i));
> > i915_ring_error_state(m, dev, &error->ring[i]);
> > @@ -1091,6 +1101,7 @@ static void i915_capture_reg_state(struct
> drm_i915_private *dev_priv,
> > struct drm_i915_error_state *error)
> > {
> > struct drm_device *dev = dev_priv->dev;
> > + int i, pipe;
> >
> > /* General organization
> > * 1. Registers specific to a single generation
> > @@ -1135,7 +1146,13 @@ static void i915_capture_reg_state(struct
> drm_i915_private *dev_priv,
> > if (HAS_HW_CONTEXTS(dev))
> > error->ccid = I915_READ(CCID);
> >
> > - if (HAS_PCH_SPLIT(dev))
> > + if (IS_GEN8(dev)) {
> > + for_each_pipe(pipe)
> > + error->pipe_ier[pipe] =
> > +
> I915_READ(GEN8_DE_PIPE_IER(pipe));
> > + for (i = 0; i < 4; i++)
> > + error->gt_ier[i] = I915_READ(GEN8_GT_IER(i));
> > + } else if (HAS_PCH_SPLIT(dev))
> > error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> > else {
> > if (IS_GEN2(dev))
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6656 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv
2014-08-05 14:51 [PATCH 00/15] drm-intel-collector - update Rodrigo Vivi
` (13 preceding siblings ...)
2014-08-05 14:51 ` [PATCH 14/15] drm/i915: capture_reg_state interrupt registers for Gen8 Rodrigo Vivi
@ 2014-08-05 14:51 ` Rodrigo Vivi
2014-08-06 0:51 ` O'Rourke, Tom
14 siblings, 1 reply; 36+ messages in thread
From: Rodrigo Vivi @ 2014-08-05 14:51 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Deepak S <deepak.s@linux.intel.com>
Higher RC6 residency is observed using timeout mode
instead of EI mode. It's Recommended to use TO Method for RC6.
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d3085b7..0cc8460 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4083,7 +4083,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
I915_WRITE(GEN6_RC_SLEEP, 0);
- I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
+ I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
/* allows RC6 residency counter to work */
I915_WRITE(VLV_COUNTER_CONTROL,
@@ -4099,7 +4099,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
/* 3: Enable RC6 */
if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
(pcbr >> VLV_PCBR_ADDR_SHIFT))
- rc6_mode = GEN6_RC_CTL_EI_MODE(1);
+ rc6_mode = GEN7_RC_CTL_TO_MODE;
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv
2014-08-05 14:51 ` [PATCH 15/15] drm/i915/chv: Use timeout mode for RC6 on chv Rodrigo Vivi
@ 2014-08-06 0:51 ` O'Rourke, Tom
0 siblings, 0 replies; 36+ messages in thread
From: O'Rourke, Tom @ 2014-08-06 0:51 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Tue, Aug 05, 2014 at 07:51:26AM -0700, Rodrigo Vivi wrote:
> From: Deepak S <deepak.s@linux.intel.com>
>
> Higher RC6 residency is observed using timeout mode
> instead of EI mode. It's Recommended to use TO Method for RC6.
>
[TOR:] When I made the similar change for BDW, I understood timeout mode will
provide benefit on some pre-production CHV steppings and no benefit on the
production CHV steppings. Is that understanding still correct? Do we want
to merge a change that is not expected to benefit any production steppings?
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d3085b7..0cc8460 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4083,7 +4083,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
> I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
> I915_WRITE(GEN6_RC_SLEEP, 0);
>
> - I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
> + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
[TOR:] A comment to explain the meaning of 0x557 could be helpful.
>
> /* allows RC6 residency counter to work */
> I915_WRITE(VLV_COUNTER_CONTROL,
> @@ -4099,7 +4099,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
> /* 3: Enable RC6 */
> if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
> (pcbr >> VLV_PCBR_ADDR_SHIFT))
> - rc6_mode = GEN6_RC_CTL_EI_MODE(1);
> + rc6_mode = GEN7_RC_CTL_TO_MODE;
>
> I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>
[TOR:] If we want to change from EI mode to TO mode for CHV,
this patch does that correctly.
^ permalink raw reply [flat|nested] 36+ messages in thread