[Mesa-dev] [PATCH v5] i965: Fix ETC2/EAC GetCompressed* functions on Gen7 GPUs

[Eleni Maria Stea]

On 07/10/2018 03:10 AM, Nanley Chery wrote:
> On Thu, Jun 14, 2018 at 10:50:57PM +0300, Eleni Maria Stea wrote:
>> On 06/14/2018 10:27 PM, Nanley Chery wrote:
>>
>>> +Jason, Ken
>>>
>>> Hello,
>>>
>>> I recently did some miptree work relating to the r8stencil_mt and I
>>> think I now have a more informed opinion about how things should be
>>> structured. I'd like to propose an alternative solution.
>>>
>>> I had initially thought we should have a separate miptree to hold the
>>> compressed data, like this patch does, but now I think we should
>>> actually have the compressed data be the main miptree and to store the
>>> decompressed miptree as part of the main one. The reasoning is that we
>>> could reuse this structure to handle the r8stencil workaround and to
>>> eventually handle the ASTC_LDR surfaces that are modified on gen9.
>>>
>>> I'm proposing something like the following:
>>>
>>> 1. Rename r8stencil_mt ->shadow_mt and
>>>    r8stencil_needs_update -> shadow_needs_update.
>>> 2. Make shadow_mt hold the decompressed ETC miptree
>>> 3. Update shadow_needs_update whenever the main mt is modified
>>> 4. Add an function to update the shadow_mt using the main mt as a source
>>> 5. Sample from the shadow_mt as appropriate
>>> 6. Make the main miptree hold the compressed data
>>>
>>> This method should also be able to handle the CopyImage functions. What
>>> do you all think?
>>>
>>> -Nanley
>>
>> Hi Nanley,
>>
>> Thank you for your reply. I wasn't aware that there are other cases we
>> might need to store a 2nd image. I agree that it's more reasonable to
>> use one generic purpose miptree that can be accessible from different
>> parts of the i965 code for such cases instead of storing miptrees in
>> different places for different hacks when a feature is not supported.
>>
>> I will search your patch to get a look and I will also get a look at the
>> mesa code to see how easy this fix would be (which parts of the code it
>> might affect) and if everyone agrees that this is a good idea I will
>> modify this patch according to your suggestions.
>>
>> BR :)
>> Eleni
> 
> Hi Eleni,
> 
> I gave this more thought and am now thinking that what you have here is
> fine. Having two different ways of working with a shadow miptree
> suggests a refactor later on, but IMO this is ultimately a step in the
> right direction. Sorry for the noise.
> 
> With code-sharing among shadow miptrees in mind, my two main
> suggestions are 1) to perform mapping operations only with the cmt (if
> it's present) and 2) to update the decompressed mt, on demand. Maybe
> with intel_miptree_copy_slice_sw?
> 
> Regards,
> Nanley
> 

Hi Nanley,

I talked to you on IRC but I reply here as well:

Thank you for the suggestions, I had misunderstood something from our
IRC conversation that followed this e-mail, so the patch v6 has several
issues. I will send a new one soon and I will implement the solution you
suggested earlier (suggestions 1-6) instead. Sorry for the noise with
the patch v6.

Thanks,
Eleni