On 13.09.2017 08:26, Denis Pauk wrote:
Little additional note:
This checks related to possible issue with such kind of supported format
check in external code:
----
for(int i=0; i<PIPE_FORMAT_COUNT + 10; i++) {
pscreen->is_format_supported(pscreen, i, PIPE_TEXTURE_2D, 0,
PIPE_BIND_SAMPLER_VIEW);
}
-----
When we have segfault with: 73, 78, 79, 80, 81,
86 (holes in enum pipe_format) and format value
bigger than PIPE_FORMAT_COUNT.
Arguably, that code should call util_format_description() and skip the
format is the result is NULL.
And why on earth is that code looping until PIPE_FORMAT_COUNT + 10 in
the first place? That never makes sense.
Cheers,
Nicolai
Best regards,
Denis.
On Sep 13, 2017 7:54 AM, "Денис Паук" <pauk.de...@gmail.com
<mailto:pauk.de...@gmail.com>> wrote:
Do you mean delete check in u_format.c:: util_format_is_supported?
Could you please explain more?
On Wed, Sep 13, 2017 at 1:32 AM, Marek Olšák <mar...@gmail.com
<mailto:mar...@gmail.com>> wrote:
On Wed, Sep 13, 2017 at 12:31 AM, Marek Olšák <mar...@gmail.com
<mailto:mar...@gmail.com>> wrote:
> I think we shouldn't be getting PIPE_FORMAT_COUNT in
> is_format_supported in the first place, and therefore drivers don't
> have to work around it.
Or any other invalid formats, for that matter.
Marek
>
> Marek
>
> On Tue, Sep 12, 2017 at 10:38 PM, Denis Pauk
<pauk.de...@gmail.com <mailto:pauk.de...@gmail.com>> wrote:
>> Bugzilla:
https://bugs.freedesktop.org/show_bug.cgi?id=102552
<https://bugs.freedesktop.org/show_bug.cgi?id=102552>
>>
>> v2: Patch cleanup proposed by Nicolai Hähnle.
>> * deleted changes in si_translate_texformat.
>>
>> Cc: Nicolai Hähnle <nhaeh...@gmail.com
<mailto:nhaeh...@gmail.com>>
>> Cc: Ilia Mirkin <imir...@alum.mit.edu
<mailto:imir...@alum.mit.edu>>
>> ---
>> src/gallium/auxiliary/util/u_format.c | 4 ++++
>> src/gallium/drivers/r600/r600_state_common.c | 4 ++++
>> src/gallium/drivers/radeonsi/si_state.c | 10 +++++++++-
>> 3 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_format.c
b/src/gallium/auxiliary/util/u_format.c
>> index 3d281905ce..a6d42a428d 100644
>> --- a/src/gallium/auxiliary/util/u_format.c
>> +++ b/src/gallium/auxiliary/util/u_format.c
>> @@ -238,6 +238,10 @@ util_format_is_subsampled_422(enum
pipe_format format)
>> boolean
>> util_format_is_supported(enum pipe_format format, unsigned
bind)
>> {
>> + if (format >= PIPE_FORMAT_COUNT) {
>> + return FALSE;
>> + }
>> +
>> if (util_format_is_s3tc(format) &&
!util_format_s3tc_enabled) {
>> return FALSE;
>> }
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c
b/src/gallium/drivers/r600/r600_state_common.c
>> index c1bce8304b..1515c28091 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -2284,6 +2284,8 @@ uint32_t
r600_translate_texformat(struct pipe_screen *screen,
>> format = PIPE_FORMAT_A4R4_UNORM;
>>
>> desc = util_format_description(format);
>> + if (!desc)
>> + goto out_unknown;
>>
>> /* Depth and stencil swizzling is handled separately. */
>> if (desc->colorspace != UTIL_FORMAT_COLORSPACE_ZS) {
>> @@ -2650,6 +2652,8 @@ uint32_t
r600_translate_colorformat(enum chip_class chip, enum
pipe_format forma
>> const struct util_format_description *desc =
util_format_description(format);
>> int channel =
util_format_get_first_non_void_channel(format);
>> bool is_float;
>> + if (!desc)
>> + return ~0U;
>>
>> #define HAS_SIZE(x,y,z,w) \
>> (desc->channel[0].size == (x) &&
desc->channel[1].size == (y) && \
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
>> index ee070107fd..f7ee24bdc6 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -1292,6 +1292,8 @@ static void
si_emit_db_render_state(struct si_context *sctx, struct r600_atom *s
>> static uint32_t si_translate_colorformat(enum pipe_format
format)
>> {
>> const struct util_format_description *desc =
util_format_description(format);
>> + if (!desc)
>> + return V_028C70_COLOR_INVALID;
>>
>> #define HAS_SIZE(x,y,z,w) \
>> (desc->channel[0].size == (x) &&
desc->channel[1].size == (y) && \
>> @@ -1796,7 +1798,11 @@ static unsigned si_tex_dim(struct
si_screen *sscreen, struct r600_texture *rtex,
>>
>> static bool si_is_sampler_format_supported(struct
pipe_screen *screen, enum pipe_format format)
>> {
>> - return si_translate_texformat(screen, format,
util_format_description(format),
>> + struct util_format_description *desc =
util_format_description(format);
>> + if (!desc)
>> + return false;
>> +
>> + return si_translate_texformat(screen, format, desc,
>>
util_format_get_first_non_void_channel(format)) != ~0U;
>> }
>>
>> @@ -1925,6 +1931,8 @@ static unsigned
si_is_vertex_format_supported(struct pipe_screen *screen,
>> PIPE_BIND_VERTEX_BUFFER)) == 0);
>>
>> desc = util_format_description(format);
>> + if (!desc)
>> + return 0;
>>
>> /* There are no native 8_8_8 or 16_16_16 data
formats, and we currently
>> * select 8_8_8_8 and 16_16_16_16 instead. This
works reasonably well
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
<mailto:mesa-dev@lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
<https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
--
Best regards,
Denis.
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
