[Intel-gfx] [PATCH v9 1/2] drm/i915: Implement WaProgramMgsrForCorrectSliceSpecificMmioReads

[Oscar Mateo]

On 4/18/2018 9:38 AM, Chris Wilson wrote:
> Quoting Oscar Mateo (2018-04-18 17:30:41)
>>
>> On 4/17/2018 3:58 PM, Yunwei Zhang wrote:
>>> +     /*
>>> +      * HW expects MCR to be programed to a enabled slice/subslice pair
>>> +      * before any MMIO read into slice/subslice register
>>> +      */
>> The comment above makes more sense in sanitize_mcr, together with the WA
>> label. You can make it a bit more verbose with the info in the commit
>> message. Something like this:
>>
>> /*
>>    * WaProgramMgsrForCorrectSliceSpecificMmioReads:cnl,icl
>>    * Before any MMIO read into slice/subslice specific registers, MCR
>>    * packet control register needs to be programmed to point to any
>>    * enabled s/ss pair. Otherwise, incorrect values will be returned.
>>    * This means each subsequent MMIO read will be forwarded to an
>>    * specific s/ss combination, but this is OK since these registers
>>    * are consistent across s/ss in almost all cases. In the rare
>>    * occasions, such as INSTDONE, where this value is dependent
>>    * on s/ss combo, the read shoud be done with read_subslice_reg.
>>    */
>>
>> I don't think any other comment is required here.
> Apart from the answer to the earlier question, what mmio read after this
> point? If all slice/subslice register access is through this function,
> what are you trying to protect? Very curious.
> -Chris

The problem is that the BSpec does not have a comprehensive list of 
registers that live in the slice/subslice, so it's difficult to know 
when this is going to become a problem. For example, I know from 
previous experience that the MOCS tables are replicated across slices 
(that's why we couldn't let UMD decide when to shutdown them: because 
the MOCS tables get lost as soon as you reconfigure the number of s/ss. 
The only way to do this in by poking in the context, so that the MOCS 
gets reprogrammed immediately after).