Re: [PATCH v2] nv50: enable texture compression

[Tobias Klausmann <tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org>]

On 03.01.2015 00:20, Ilia Mirkin wrote:
> On Fri, Jan 2, 2015 at 6:09 PM, Tobias Klausmann
> <tobias.johannes.klausmann@mni.thm.de> wrote:
>> We enable compression only for some supported formats
>>
>> Suggested-by: Ilia Mirkin <imirkin@alum.mit.edu>
>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
>> ---
>>   src/gallium/drivers/nouveau/nv50/nv50_miptree.c | 23 +++++++++++++++++++++--
>>   src/gallium/drivers/nouveau/nv50/nv50_screen.c  |  7 +++++++
>>   2 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
>> index 1aacaec..13ed8a3 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
>> @@ -65,8 +65,7 @@ nv50_tex_choose_tile_dims(unsigned nx, unsigned ny, unsigned nz)
>>   static uint32_t
>>   nv50_mt_choose_storage_type(struct nv50_miptree *mt, boolean compressed)
> The caller of this currently always says true. However it should
> probably be drm_version >= whatever.
Oh sure, i'll add it!

>
>>   {
>> -   const unsigned ms = mt->ms_x + mt->ms_y;
>> -
>> +   const unsigned ms = util_logbase2(mt->base.base.nr_samples);
>>      uint32_t tile_flags;
>>
>>      if (unlikely(mt->base.base.flags & NOUVEAU_RESOURCE_FLAG_LINEAR))
>> @@ -96,6 +95,26 @@ nv50_mt_choose_storage_type(struct nv50_miptree *mt, boolean compressed)
>>         tile_flags = 0x60 + ms;
>>         break;
>>      default:
>> +      switch (mt->base.base.format) {
> You're already switching on the format. You could be all clever and do
>
> default:
>    compressed = false;
>    /* fallthrough */
> case PIPE_FORMAT_A:
> case PIPE_FORMAT_B:
>    switch (blocksize) { ... }
>
> It's a bit unusual to have the default case in the middle, but I'm
> pretty sure it works.
We talked about broken compilers, hope we don't hit one here :D

>
> BTW, I assume that there was no dmesg spam or piglit regressions with
> this change?
Indeed!

>
>    -ilia

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau