[Mesa-dev] [PATCH v2 07/42] i965: Implement programming of the L3 configuration.

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On Monday, November 23, 2015 11:09:22 PM Jordan Justen wrote:
>> On 2015-11-17 21:54:39, Jordan Justen wrote:
>> > From: Francisco Jerez <currojerez at riseup.net>
>> > 
>> > ---
>> >  src/mesa/drivers/dri/i965/gen7_l3_state.c | 95 +++++++++++++++++++++++++++++++
>> >  1 file changed, 95 insertions(+)
>> > 
>> > diff --git a/src/mesa/drivers/dri/i965/gen7_l3_state.c b/src/mesa/drivers/dri/i965/gen7_l3_state.c
>> > index 8f9ba5b..48bca29 100644
>> > --- a/src/mesa/drivers/dri/i965/gen7_l3_state.c
>> > +++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c
>> > @@ -161,3 +161,98 @@ get_l3_way_size(const struct brw_device_info *devinfo)
>> >     else
>> >        return 2 << devinfo->gt;
>> >  }
>> > +
>> > +/**
>> > + * Program the hardware to use the specified L3 configuration.
>> > + */
>> > +static void
>> > +setup_l3_config(struct brw_context *brw, const struct brw_l3_config *cfg)
>> > +{
>> > +   const bool has_dc = cfg->n[L3P_DC] || cfg->n[L3P_ALL];
>> > +   const bool has_is = cfg->n[L3P_IS] || cfg->n[L3P_RO] || cfg->n[L3P_ALL];
>> > +   const bool has_c = cfg->n[L3P_C] || cfg->n[L3P_RO] || cfg->n[L3P_ALL];
>> > +   const bool has_t = cfg->n[L3P_T] || cfg->n[L3P_RO] || cfg->n[L3P_ALL];
>> > +   const bool has_slm = cfg->n[L3P_SLM];
>> > +
>> > +   /* According to the hardware docs, the L3 partitioning can only be changed
>> > +    * while the pipeline is completely drained and the caches are flushed,
>> > +    * what involves a first PIPE_CONTROL flush which stalls the pipeline and
>> > +    * initiates invalidation of the relevant caches...
>> > +    */
>> > +   brw_emit_pipe_control_flush(brw,
>> > +                               PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
>> > +                               PIPE_CONTROL_CONST_CACHE_INVALIDATE |
>> > +                               PIPE_CONTROL_INSTRUCTION_INVALIDATE |
>> > +                               PIPE_CONTROL_DATA_CACHE_INVALIDATE |
>> > +                               PIPE_CONTROL_NO_WRITE |
>> > +                               PIPE_CONTROL_CS_STALL);
>> > +
>> > +   /* ...followed by a second stalling flush which guarantees that
>> > +    * invalidation is complete when the L3 configuration registers are
>> > +    * modified.
>> > +    */
>> > +   brw_emit_pipe_control_flush(brw,
>> > +                               PIPE_CONTROL_DATA_CACHE_INVALIDATE |
>> > +                               PIPE_CONTROL_NO_WRITE |
>> > +                               PIPE_CONTROL_CS_STALL);
>> 
>> On Ivy Bridge, these two pipe control calls are causing the simulator
>> to complain. The message seems to indicate that, essentially the
>> gen8_add_cs_stall_workaround_bits workaround is not being applied.
>> 
>> Looking at the IVB PRM, vol 2, part 1, "1.10.4.1 PIPE_CONTROL" under
>> bit20 for "CS Stall" is does seem to indicate that CS Stall also
>> requires 1 of several other bits to be added.
>> 
>> For example, adding PIPE_CONTROL_STALL_AT_SCOREBOARD to this call, and
>> the one below makes the simulator happy. But, perhaps
>> gen8_add_cs_stall_workaround_bits should be applied to SNB - HSW as
>> well?
>> 
>> -Jordan
>
> I would definitely ack a patch to do gen8_add_cs_stall_workaround_bits
> on Gen6-8 and rename it to gen6_add_cs_stall_workaround_bits.  AFAICT
> it's always been needed; we just missed it and I was being conservative
> during the Broadwell enabling.
>

Yeah, we should definitely fix that: We're applying the workaround to
Gen8+ while according to the docs it should only be applied to
Gen6-Gen8.

> --Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151124/eddd523d/attachment.sig>