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

[Ville Syrjälä]

On Wed, Jun 03, 2015 at 10:05:25AM +0300, Abdiel Janulgue wrote:
> 
> 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.

I suppose you need to at least set the RS context enable bit in the
RING_CONTEXT_CONTROL register (in populate_lr_context()).

-- 
Ville Syrjälä
Intel OTC