Intel-GFX Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
@ 2011-06-07 22:54 Kenneth Graunke
  2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Kenneth Graunke @ 2011-06-07 22:54 UTC (permalink / raw)
  To: intel-gfx

According to BSpec volume 1c.4 section 3.2.9, Display (Plane) Select is
now at bits 21:19 instead of 21:20.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/intel_display.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 81a9059..60b2941 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6390,7 +6390,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		break;
 
 	case 6:
-	case 7:
 		OUT_RING(MI_DISPLAY_FLIP |
 			 MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 		OUT_RING(fb->pitch | obj->tiling_mode);
@@ -6400,6 +6399,16 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		pipesrc = I915_READ(PIPESRC(pipe)) & 0x0fff0fff;
 		OUT_RING(pf | pipesrc);
 		break;
+
+	case 7:
+		OUT_RING(MI_DISPLAY_FLIP | (intel_crtc->plane << 19));
+		OUT_RING(fb->pitch | obj->tiling_mode);
+		OUT_RING(obj->gtt_offset);
+
+		pf = I915_READ(PF_CTL(pipe)) & PF_ENABLE;
+		pipesrc = I915_READ(PIPESRC(pipe)) & 0x0fff0fff;
+		OUT_RING(pf | pipesrc);
+		break;
 	}
 	ADVANCE_LP_RING();
 
-- 
1.7.4.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall.
  2011-06-07 22:54 [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge Kenneth Graunke
@ 2011-06-07 22:54 ` Kenneth Graunke
  2011-06-07 23:16   ` Keith Packard
  2011-06-08  9:29   ` Chris Wilson
  2011-06-07 22:54 ` [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge Kenneth Graunke
  2011-06-07 23:14 ` [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset " Keith Packard
  2 siblings, 2 replies; 12+ messages in thread
From: Kenneth Graunke @ 2011-06-07 22:54 UTC (permalink / raw)
  To: intel-gfx

Ivybridge has BLT and BSD rings, so there's no need to check.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/i915_irq.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b9fafe3..6a2b7f4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1841,10 +1841,8 @@ int ivybridge_irq_postinstall(struct drm_device *dev)
 	u32 hotplug_mask;
 
 	DRM_INIT_WAITQUEUE(&dev_priv->ring[RCS].irq_queue);
-	if (HAS_BSD(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
-	if (HAS_BLT(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
+	DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
+	DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
 
 	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
 	dev_priv->irq_mask = ~display_mask;
-- 
1.7.4.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge.
  2011-06-07 22:54 [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge Kenneth Graunke
  2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
@ 2011-06-07 22:54 ` Kenneth Graunke
  2011-06-07 23:18   ` Keith Packard
  2011-06-07 23:14 ` [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset " Keith Packard
  2 siblings, 1 reply; 12+ messages in thread
From: Kenneth Graunke @ 2011-06-07 22:54 UTC (permalink / raw)
  To: intel-gfx

According to the hardware documentation, GDRST is exactly the same as on
Sandybridge.  So simply enable the existing code.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/i915_drv.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0defd42..b641a64 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -577,6 +577,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	if (get_seconds() - dev_priv->last_gpu_reset < 5) {
 		DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
 	} else switch (INTEL_INFO(dev)->gen) {
+	case 7:
 	case 6:
 		ret = gen6_do_reset(dev, flags);
 		break;
-- 
1.7.4.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-07 22:54 [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge Kenneth Graunke
  2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
  2011-06-07 22:54 ` [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge Kenneth Graunke
@ 2011-06-07 23:14 ` Keith Packard
  2011-06-08  0:55   ` Kenneth Graunke
  2 siblings, 1 reply; 12+ messages in thread
From: Keith Packard @ 2011-06-07 23:14 UTC (permalink / raw)
  To: Kenneth Graunke, intel-gfx

[-- Attachment #1.1: Type: text/plain, Size: 937 bytes --]

On Tue,  7 Jun 2011 15:54:39 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> According to BSpec volume 1c.4 section 3.2.9, Display (Plane) Select is
> now at bits 21:19 instead of 21:20.
> 
> Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>

I will note that the docs have an obvious bug -- 21:8 are 'reserved' on
IVB while 21:19 are 'Display (Plane) Select'. I trust you've actually
tried this on hardware and noticed that it works better now?


> +
> +	case 7:
> +		OUT_RING(MI_DISPLAY_FLIP | (intel_crtc->plane << 19));
> +		OUT_RING(fb->pitch | obj->tiling_mode);
> +		OUT_RING(obj->gtt_offset);
> +
> +		pf = I915_READ(PF_CTL(pipe)) & PF_ENABLE;
> +		pipesrc = I915_READ(PIPESRC(pipe)) & 0x0fff0fff;
> +		OUT_RING(pf | pipesrc);

What's this last DWORD supposed to be for? The IVB spec says length
should be '1' and there should be only 3 DWORDS in this command.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall.
  2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
@ 2011-06-07 23:16   ` Keith Packard
  2011-06-08  9:29   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Keith Packard @ 2011-06-07 23:16 UTC (permalink / raw)
  To: Kenneth Graunke, intel-gfx

[-- Attachment #1.1: Type: text/plain, Size: 231 bytes --]

On Tue,  7 Jun 2011 15:54:40 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> Ivybridge has BLT and BSD rings, so there's no need to check.

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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] 12+ messages in thread

* Re: [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge.
  2011-06-07 22:54 ` [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge Kenneth Graunke
@ 2011-06-07 23:18   ` Keith Packard
  0 siblings, 0 replies; 12+ messages in thread
From: Keith Packard @ 2011-06-07 23:18 UTC (permalink / raw)
  To: Kenneth Graunke, intel-gfx

[-- Attachment #1.1: Type: text/plain, Size: 297 bytes --]

On Tue,  7 Jun 2011 15:54:41 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:

> According to the hardware documentation, GDRST is exactly the same as on
> Sandybridge.  So simply enable the existing code.

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-07 23:14 ` [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset " Keith Packard
@ 2011-06-08  0:55   ` Kenneth Graunke
  2011-06-08  1:09     ` Keith Packard
  2011-06-08  9:36     ` Chris Wilson
  0 siblings, 2 replies; 12+ messages in thread
From: Kenneth Graunke @ 2011-06-08  0:55 UTC (permalink / raw)
  To: Keith Packard; +Cc: intel-gfx

On 06/07/2011 04:14 PM, Keith Packard wrote:
> On Tue,  7 Jun 2011 15:54:39 -0700, Kenneth Graunke<kenneth@whitecape.org>  wrote:
>> According to BSpec volume 1c.4 section 3.2.9, Display (Plane) Select is
>> now at bits 21:19 instead of 21:20.
>>
>> Signed-off-by: Kenneth Graunke<kenneth@whitecape.org>
>
> I will note that the docs have an obvious bug -- 21:8 are 'reserved' on
> IVB while 21:19 are 'Display (Plane) Select'.

In the latest version of the Render Engine Command Streamer chapter, 
18:8 are 'reserved' on IVB.  Perhaps you have an old copy?

Apparently MI_DISPLAY_FLIP also exists in the Blitter Engine on IVB.  I 
suspect we actually need to use that, but I haven't tried to yet.

> I trust you've actually
> tried this on hardware and noticed that it works better now?

No, actually...page flips are still broken.  I suspect this patch is 
necessary but insufficient.  Feel free to hold off on merging it.

>> +
>> +	case 7:
>> +		OUT_RING(MI_DISPLAY_FLIP | (intel_crtc->plane<<  19));
>> +		OUT_RING(fb->pitch | obj->tiling_mode);
>> +		OUT_RING(obj->gtt_offset);
>> +
>> +		pf = I915_READ(PF_CTL(pipe))&  PF_ENABLE;
>> +		pipesrc = I915_READ(PIPESRC(pipe))&  0x0fff0fff;
>> +		OUT_RING(pf | pipesrc);
>
> What's this last DWORD supposed to be for? The IVB spec says length
> should be '1' and there should be only 3 DWORDS in this command.

Good question.  My reading of the docs say that it should be 3 DWORDs on 
SNB as well; this code was directly lifted from "case 6" above (save the 
first line).

--Kenneth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-08  0:55   ` Kenneth Graunke
@ 2011-06-08  1:09     ` Keith Packard
  2011-06-08  9:36     ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Keith Packard @ 2011-06-08  1:09 UTC (permalink / raw)
  To: Kenneth Graunke; +Cc: intel-gfx

[-- Attachment #1.1: Type: text/plain, Size: 972 bytes --]

On Tue, 07 Jun 2011 17:55:05 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:

> In the latest version of the Render Engine Command Streamer chapter, 
> 18:8 are 'reserved' on IVB.  Perhaps you have an old copy?

Yeah, I haven't sync'd for a week or so.

> Apparently MI_DISPLAY_FLIP also exists in the Blitter Engine on IVB.  I 
> suspect we actually need to use that, but I haven't tried to yet.

> No, actually...page flips are still broken.  I suspect this patch is 
> necessary but insufficient.  Feel free to hold off on merging it.

Sounds like a bit more testing and fixing is required; I'll leave things
alone until we know how to make it work.

> Good question.  My reading of the docs say that it should be 3 DWORDs on 
> SNB as well; this code was directly lifted from "case 6" above (save the 
> first line).

Might be interesting to see if page flipping works when we send the
command correctly :-)

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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] 12+ messages in thread

* Re: [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall.
  2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
  2011-06-07 23:16   ` Keith Packard
@ 2011-06-08  9:29   ` Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2011-06-08  9:29 UTC (permalink / raw)
  To: Kenneth Graunke, intel-gfx

On Tue,  7 Jun 2011 15:54:40 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> Ivybridge has BLT and BSD rings, so there's no need to check.

Right, I've been trying to move this code into the ring init...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-08  0:55   ` Kenneth Graunke
  2011-06-08  1:09     ` Keith Packard
@ 2011-06-08  9:36     ` Chris Wilson
  2011-06-08  9:54       ` Kenneth Graunke
  1 sibling, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2011-06-08  9:36 UTC (permalink / raw)
  To: Kenneth Graunke, Keith Packard; +Cc: intel-gfx

On Tue, 07 Jun 2011 17:55:05 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> On 06/07/2011 04:14 PM, Keith Packard wrote:
> > What's this last DWORD supposed to be for? The IVB spec says length
> > should be '1' and there should be only 3 DWORDS in this command.
> 
> Good question.  My reading of the docs say that it should be 3 DWORDs on 
> SNB as well; this code was directly lifted from "case 6" above (save the 
> first line).

In my ancient copy of the specs, MI_DISPLAY_FLIP is 4 dwords, with this
last dword for programming the panel-fitter and pipesrc. And it also
says that command is applicable to IVB, but as I said it is an ancient
copy.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-08  9:36     ` Chris Wilson
@ 2011-06-08  9:54       ` Kenneth Graunke
  2011-06-08 10:57         ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Kenneth Graunke @ 2011-06-08  9:54 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On 06/08/2011 02:36 AM, Chris Wilson wrote:
> On Tue, 07 Jun 2011 17:55:05 -0700, Kenneth Graunke<kenneth@whitecape.org>  wrote:
>> On 06/07/2011 04:14 PM, Keith Packard wrote:
>>> What's this last DWORD supposed to be for? The IVB spec says length
>>> should be '1' and there should be only 3 DWORDS in this command.
>>
>> Good question.  My reading of the docs say that it should be 3 DWORDs on
>> SNB as well; this code was directly lifted from "case 6" above (save the
>> first line).
>
> In my ancient copy of the specs, MI_DISPLAY_FLIP is 4 dwords, with this
> last dword for programming the panel-fitter and pipesrc. And it also
> says that command is applicable to IVB, but as I said it is an ancient
> copy.
> -Chris

That's correct.  SNB /does/ include a 4th DWord as you describe---newer 
specs got mangled and lost that info, somehow.  Argh.

IVB does only use three DWords, so the last OUT_RING should be nixed. 
Removing that does change the behavior, but doesn't yet make it work.

Thanks!
--Kenneth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge.
  2011-06-08  9:54       ` Kenneth Graunke
@ 2011-06-08 10:57         ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2011-06-08 10:57 UTC (permalink / raw)
  To: Kenneth Graunke; +Cc: intel-gfx

On Wed, 08 Jun 2011 02:54:36 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> IVB does only use three DWords, so the last OUT_RING should be nixed. 
> Removing that does change the behavior, but doesn't yet make it work.

Note you can't just drop it since we need to keep the quadword
alignment, so shrink the command and add a trailing nop.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-07 22:54 [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset on Ivybridge Kenneth Graunke
2011-06-07 22:54 ` [PATCH 2/3] drm/i915: Remove HAS_BLT/HAS_BSD checks from ivybridge_irq_postinstall Kenneth Graunke
2011-06-07 23:16   ` Keith Packard
2011-06-08  9:29   ` Chris Wilson
2011-06-07 22:54 ` [PATCH 3/3] drm/i915: Enable GPU reset on Ivybridge Kenneth Graunke
2011-06-07 23:18   ` Keith Packard
2011-06-07 23:14 ` [PATCH 1/3] drm/i915: Fix MI_DISPLAY_FLIP plane select offset " Keith Packard
2011-06-08  0:55   ` Kenneth Graunke
2011-06-08  1:09     ` Keith Packard
2011-06-08  9:36     ` Chris Wilson
2011-06-08  9:54       ` Kenneth Graunke
2011-06-08 10:57         ` Chris Wilson

Intel-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/intel-gfx/0 intel-gfx/git/0.git
	git clone --mirror https://lore.kernel.org/intel-gfx/1 intel-gfx/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 intel-gfx intel-gfx/ https://lore.kernel.org/intel-gfx \
		intel-gfx@lists.freedesktop.org
	public-inbox-index intel-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.intel-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git