[PATCH] uxa: Support BLT ring flushes on Broadwell, but not render ring flushes.

[PATCH] uxa: Support BLT ring flushes on Broadwell, but not render ring flushes.

[Kenneth Graunke]

Several places (such as intel_cache_expire) call intel_emit_batch_flush,
so it needs to work on Broadwell.  Sometimes the batch is empty, in
which case current_batch may not yet be BLT_RING.

The PIPE_CONTROL code has not been ported to work on Broadwell, so
trying to do a render ring flush will hang the GPU.  It also doesn't
make any sense to do a render ring flush, given that we never use the
render ring for UXA on Broadwell.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 src/uxa/intel_batchbuffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

This is an alternative to my old "uxa: Don't emit PIPE_CONTROLs in an
empty batch." patch, which Chris pointed out was broken.

Chris,

In the future, if you're going to rewrite significant portions of my
patches, could you please at least put your Signed-off-by or something
on it?  In the version of "uxa: Enable BLT acceleration on Broadwell.",
you committed, at least half the patch was not actually written by me,
and the resulting code either hit assertion failures or GPU hangs if
run at all.

It's pretty disconcerting to see code committed under my name, with my
Signed-off-by, that doesn't work and which I've never even seen before.

diff --git a/src/uxa/intel_batchbuffer.c b/src/uxa/intel_batchbuffer.c
index 4aabe48..ec71ce2 100644
--- a/src/uxa/intel_batchbuffer.c
+++ b/src/uxa/intel_batchbuffer.c
@@ -183,11 +183,11 @@ void intel_batch_emit_flush(ScrnInfoPtr scrn)
 	int flags;
 
 	assert (!intel->in_batch_atomic);
-	assert (INTEL_INFO(intel)->gen < 0100);
 
 	/* Big hammer, look to the pipelined flushes in future. */
 	if ((INTEL_INFO(intel)->gen >= 060)) {
-		if (intel->current_batch == BLT_BATCH) {
+		if (intel->current_batch == BLT_BATCH ||
+		    INTEL_INFO(intel)->gen >= 0100) {
 			BEGIN_BATCH_BLT(4);
 			OUT_BATCH(MI_FLUSH_DW | 2);
 			OUT_BATCH(0);
-- 
1.8.4.2

Re: [PATCH] uxa: Support BLT ring flushes on Broadwell, but not render ring flushes.

[Chris Wilson]

On Mon, Mar 17, 2014 at 09:27:16AM -0700, Kenneth Graunke wrote:
> Chris,
> 
> In the future, if you're going to rewrite significant portions of my
> patches, could you please at least put your Signed-off-by or something
> on it?  In the version of "uxa: Enable BLT acceleration on Broadwell.",
> you committed, at least half the patch was not actually written by me,
> and the resulting code either hit assertion failures or GPU hangs if
> run at all.
> 
> It's pretty disconcerting to see code committed under my name, with my
> Signed-off-by, that doesn't work and which I've never even seen before.

I do apologise that you felt I made substantive changes to the patch. As
far I was concerned the addition of the libdrm_intel version bump in
configure (a vital build fix), the change in if-else cascade (cosmetic)
and the only functional change of disabling TexturedVideo for gen8+ were
trivial.

The fact that the original patch made an assumption that was then broken
by not applying the first patch in the series was not altered by those
changes. And I should have realised that at the time.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] uxa: Support BLT ring flushes on Broadwell, but not render ring flushes.

[Kenneth Graunke]

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

On 03/18/2014 01:29 AM, Chris Wilson wrote:
> On Mon, Mar 17, 2014 at 09:27:16AM -0700, Kenneth Graunke wrote:
>> Chris,
>>
>> In the future, if you're going to rewrite significant portions of my
>> patches, could you please at least put your Signed-off-by or something
>> on it?  In the version of "uxa: Enable BLT acceleration on Broadwell.",
>> you committed, at least half the patch was not actually written by me,
>> and the resulting code either hit assertion failures or GPU hangs if
>> run at all.
>>
>> It's pretty disconcerting to see code committed under my name, with my
>> Signed-off-by, that doesn't work and which I've never even seen before.
> 
> I do apologise that you felt I made substantive changes to the patch. As
> far I was concerned the addition of the libdrm_intel version bump in
> configure (a vital build fix), the change in if-else cascade (cosmetic)
> and the only functional change of disabling TexturedVideo for gen8+ were
> trivial.
> 
> The fact that the original patch made an assumption that was then broken
> by not applying the first patch in the series was not altered by those
> changes. And I should have realised that at the time.
> -Chris

Definitely thanks for those fixes!  I should've remembered the libdrm
requirement bump.  Also, thanks for taking the patches.

--Ken


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 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