On 4 May 2017 at 14:43, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Emil,
>
> On 4 May 2017 at 13:27, Emil Velikov <emil.l.veli...@gmail.com> wrote:
>>> @@ -581,21 +600,17 @@ intel_create_image_common(__DRIscreen *dri_screen,
>>> assert(!(use && count));
>>>
>>> uint64_t modifier = select_best_modifier(&screen->devinfo, modifiers,
>>> count);
>>> - switch (modifier) {
>>> - case I915_FORMAT_MOD_X_TILED:
>>> - assert(tiling == I915_TILING_X);
>>> - break;
>>> - case DRM_FORMAT_MOD_LINEAR:
>>> - tiling = I915_TILING_NONE;
>>> - break;
>>> - case I915_FORMAT_MOD_Y_TILED:
>>> - tiling = I915_TILING_Y;
>>> - break;
>>> - case DRM_FORMAT_MOD_INVALID:
>>> + if (modifier == DRM_FORMAT_MOD_INVALID) {
>>> + /* User requested specific modifiers, none of which work */
>>> if (modifiers)
>>> return NULL;
>>> - default:
>>> - break;
>> Originally, here we'll use I915_TILING_X...
>>
>>> +
>>> + /* Historically, X-tiled was the default, and so lack of modifier
>>> means
>>> + * X-tiled.
>>> + */
>>> + tiling = I915_TILING_X;
>>> + } else {
>>> + tiling = modifier_to_tiling(modifier);
>> ... while now we'll get I915_TILING_NONE.
>
> I don't think so ... ?
>
> If we don't find a modifier to use (LINEAR/X_TILED/Y_TILED) from
> select_best_modifier(), then we return INVALID, which hits the first
> branch above: failure if the user provided a set of modifiers we don't
> support, or TILING_X if the user didn't provide a set of modifiers. If
> we hit this branch here, then the user has explicitly requested one of
> the three modifiers we support.
>
Thanks for the correction Dan. Having a look in select_best_modifier()
would have helped ;-)
> IOW, there is no change as far as I can see, but perhaps for the
> meantime, we could use an unreachable() at the bottom of
> modifier_to_tiling(). Would that help?
>
Yes the default statement and return I915_TILING_NONE in
modifier_to_tiling() seem unreachable.
unreachable or assert - I am leaning slightly towards the latter since
it will catch fallouts in the future.
I'll leave the decision to you and the Intel devs.
In either case:
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
