On Tue, Oct 29, 2013 at 9:29 PM, Paul Berry <stereotype...@gmail.com> wrote:
> On 29 October 2013 21:28, Paul Berry <stereotype...@gmail.com> wrote:
>
>> On 29 October 2013 17:16, Anuj Phogat <anuj.pho...@gmail.com> wrote:
>>
>>>
>>>
>>>
>>> On Tue, Oct 29, 2013 at 4:31 PM, Paul Berry <stereotype...@gmail.com>wrote:
>>>
>>>
>>>> I think the right thing to do is to add:
>>>>
>>>> if (dispatch_width == 16)
>>>> fail("...");
>>>>
>>>> to whatever parts of the visitor you aren't confident will work
>>>> properly in SIMD16.
>>>>
>>> Yes. But that only covers the per sample shading enabled due to
>>> gl_SampleID and gl_SamplePosition.
>>>
>>> It still doesn't cover the case when there's no gl_SampleID or
>>> gl_SamplePosition in fragment shader but GL_SAMPLE_SHADING is enabled.
>>> To disable simd16 for this case I either need to keep the relevant
>>> changes I made in gen7_wm_state.c
>>> or
>>> may be set simd16_instructions = null in brw_wm_fs_emit() when
>>> GL_SAMPLE_SHADING is enabled. It should also be safe to
>>> call _mesa_get_min_invocations_per_fragment() in brw_wm_fs_emit(). So, we
>>> can take care of all the cases of sample shading enabled at the same place?
>>>
>>
>> Oh, I didn't realize that you were getting hangs with SIMD16 mode even
>> when neither gl_SampleID nor gl_SamplePosition is used.
>>
>> A large part of the reason I had suggested putting calls to fail() in
>> brw_fs_visitor was because I thought the hangs were due to improper code
>> generation. Since you're getting hangs just by using GL_SAMPLE_SHADING,
>> the cause mustn't be improper code generation, so my idea of failing out of
>> SIMD16 compilation doesn't make sense anymore. Based on this information I
>> guess I'm ok with the patch as originally written.
>>
>
>
> BTW, on the off chance that it helps you get unstuck with your SIMD16
> hangs, have you noticed that you have to change KSP[0] (kernel start
> pointer 0) in order to get "SIMD16 only" mode to work? Currently we always
> emit 3DSTATE_PS like this:
>
> BEGIN_BATCH(8);
> OUT_BATCH(_3DSTATE_PS << 16 | (8 - 2));
> OUT_BATCH(brw->wm.base.prog_offset); /* KSP[0] */
> OUT_BATCH(dw2);
> OUT_BATCH(dw3);
> OUT_BATCH(dw4);
> OUT_BATCH(dw5);
> OUT_BATCH(0); /* KSP[1] */
> OUT_BATCH(brw->wm.base.prog_offset +
> brw->wm.prog_data->prog_offset_16); /* KSP[2] */
> ADVANCE_BATCH();
>
> That's always worked in the past because (see Graphics BSpec:
> 3D-Media-GPGPU Engine > 3D Pipeline Stages > Pixel > Pixel Shader Thread
> Generation > Pixel Grouping (Dispatch Size) Control):
> - In "SIMD8 only" mode, KSP[0] is the SIMD8 program, and KSP[1] and KSP[2]
> are ignored.
> - In "SIMD8+SIMD16" mode, KSP[0] is the SIMD8 program, KSP[1] is ignored,
> and KSP[2] is the SIMD16 program.
>
> However, in "SIMD16 only" mode, KSP[0] needs to be the SIMD16 program.
>
> (my apologies if you've already accounted for this--it just seems like a
> likely explanation for "SIMD16 only" GPU hangs)
>
Last time I tried this didn' work well probably due to other bugs in my
implementation. But it worked this time :). Thanks for making me try it.
Although I've already received r-b on all of my patches, I'll soon send out
V3 of my series with required changes to enable SIMD16 as well.
I'll do a full piglit run to verify the changes.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
