[Mesa-dev] [PATCH resend 7/7] i965: Disable HW-binding tables on batch finish for Broadwell

[Abdiel Janulgue]

On 06/02/2015 08:28 PM, Kenneth Graunke wrote:
> On Tuesday, June 02, 2015 03:23:35 PM Abdiel Janulgue wrote:
>>
>> On 06/02/2015 09:31 AM, Kenneth Graunke wrote:
>>> On Monday, June 01, 2015 03:14:30 PM Abdiel Janulgue wrote:
>>>> This is needed since kernel doesn't support RS context save and
>>>> restore on BDW yet. So manually disable hw-generated binding tables
>>>> when done using it in the batch. Otherwise the GPU would no longer
>>>> accept software binding tables submitted by other clients including
>>>> but not limited to the Xorg driver.
>>>>
>>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>>>> ---
>>>>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 11 +++++++++++
>>>>  src/mesa/drivers/dri/i965/intel_batchbuffer.h |  3 ++-
>>>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> This seems fairly awful.  The kernel should prevent userspace from
>>> breaking other userspace...in the hardware context world, this really
>>> doesn't feel like our job.
>>>
>>> Why didn't you just update your kernel patch for Broadwell?  i.e. make
>>>
>>> drm/i915: Enable Resource Streamer state save/restore in HSW
>>>
>>> do:
>>> +       if (IS_HASWELL(ring->dev) || INTEL_INFO(ring->dev)->gen >= 8)
>>>
>>> instead of:
>>> +       if (IS_HASWELL(ring->dev))
>>>
>>> It looks like the MI_SET_CONTEXT RS save/restore bits you used on
>>> Haswell still exist on Broadwell.  Do they not work or something?
>>>
>>>
>>
>> I was hoping to have a follow-up for GEN8 as well once the initial
>> kernel patches get merged  :)
> 
> We should do it all at once - it should be trivial to support both.
> 

I put in the necessary MI_SET_CONTEXT bits for GEN8 and tried it out
this morning. Unfortunately, it doesn't work and hung my system.

I guess the reason for this is that MI_SET_CONTEXT is not used in GEN8
anymore? I found these comments and quoting from the docs in intel_lrc.c:

"Instead of the legacy MI_SET_CONTEXT, the way you tell the GPU to
switch to a contexts is via a context execution list, ergo "Execlists".

Unfortunately, I'm not that familiar with the execlist implementation in
the kernel. I have a hunch that I have to insert somewhere in the global
default context a state that says hw-generated binding tables are
disabled but I'm not quite sure where to put it, probably need to study
it a bit.

What do you think of these couple of options I have in mind? 1) We
temporarily put this mesa workaround in place until we figure out how to
do this in the kernel and remove it once we put the proper solution into
place 2) Skip out the GEN8 implementation altogether until we find
solution for kernel?

I myself am in favor of initially to having the basic parts in place
then we can move forward from that afterwards. What do you think?

-Abdiel