[Mesa-dev] [PATCH v2 1/2] i965: Use helper function for modifier -> tiling

[Daniel Stone]

Hi Emil,

On 4 May 2017 at 13:27, Emil Velikov <emil.l.velikov at 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.

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?

Cheers,
Daniel