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

[Nanley Chery]

On Wed, Jul 18, 2018 at 05:34:13PM +0300, Eleni Maria Stea wrote:
> 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.
> 

Sounds good. By the way, I think it'd be helpful if you sent out the
solution as a series of patches (see git format-patch -<n> for example).
That way it's easier to confirm each step of the solution is correct.

-Nanley

> Thanks,
> Eleni
> 
> 
>