[Mesa-dev] [PATCH 5/8] i965/dri: Handle the linear fb modifier

[Ben Widawsky]

On 17-03-21 13:48:13, Jason Ekstrand wrote:
>On Tue, Mar 21, 2017 at 1:45 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
>
>> At image creation create a path for dealing with the linear modifier.
>> This works exactly like the old usage flags where __DRI_IMAGE_USE_LINEAR
>> was specified.
>>
>> During development of this patch series, it was decided that a lack of
>> modifier was an insufficient way to express the required modifiers. As a
>> result, 0 was repurposed to mean a modifier for a LINEAR layout.
>>
>> NOTE: This patch was added for v3 of the patch series.
>>
>> v2: Rework the algorithm for modifier selection to go from a bitmask
>> based selection to this priority value.
>>
>> Requested-by: Jason Ekstrand <jason at jlekstrand.net>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>> ---
>>  src/mesa/drivers/dri/i965/intel_screen.c | 55
>> +++++++++++++++++++++++++-------
>>  1 file changed, 44 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
>> b/src/mesa/drivers/dri/i965/intel_screen.c
>> index 26ab5a8e19..8ba90f0a0c 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -45,6 +45,10 @@
>>  #define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1)
>>  #endif
>>
>> +#ifndef DRM_FORMAT_MOD_LINEAR
>> +#define DRM_FORMAT_MOD_LINEAR 0
>> +#endif
>> +
>>  static const __DRIconfigOptionsExtension brw_config_options = {
>>     .base = { __DRI_CONFIG_OPTIONS, 1 },
>>     .xml =
>> @@ -511,13 +515,36 @@ intel_destroy_image(__DRIimage *image)
>>     free(image);
>>  }
>>
>> +enum modifier_priority {
>> +   MODIFIER_PRIORITY_INVALID = 0,
>> +   MODIFIER_PRIORITY_LINEAR,
>> +};
>> +
>> +const uint64_t priority_to_modifier[] = {
>> +   [MODIFIER_PRIORITY_INVALID] = DRM_FORMAT_MOD_INVALID,
>> +   [MODIFIER_PRIORITY_LINEAR] = DRM_FORMAT_MOD_LINEAR,
>> +};
>> +
>>  static uint64_t
>>  select_best_modifier(struct gen_device_info *devinfo,
>>                       const uint64_t *modifiers,
>>                       const unsigned count)
>>  {
>> -   /* Modifiers are not supported by this DRI driver */
>> -   return DRM_FORMAT_MOD_INVALID;
>> +
>> +   enum modifier_priority prio = MODIFIER_PRIORITY_INVALID;
>> +
>> +   for (int i = 0; i < count; i++) {
>> +      switch (modifiers[i]) {
>> +      case DRM_FORMAT_MOD_LINEAR:
>> +         prio = MAX2(prio, MODIFIER_PRIORITY_LINEAR);
>> +         break;
>> +      case DRM_FORMAT_MOD_INVALID:
>> +      default:
>> +         unreachable("Invalid modifiers specified\n");
>>
>
>How'd this sneak back in?  I think you want to just fall through and
>continue on to the next loop iteration in this case.
>
>

Yep. Sorry about that.

>> +      }
>> +   }
>> +
>> +   return priority_to_modifier[prio];
>>  }
>>
>>  static __DRIimage *
>> @@ -530,7 +557,10 @@ intel_create_image_common(__DRIscreen *dri_screen,
>>  {
>>     __DRIimage *image;
>>     struct intel_screen *screen = dri_screen->driverPrivate;
>> -   uint32_t tiling;
>> +   /* Historically, X-tiled was the default, and so lack of modifier means
>> +    * X-tiled.
>> +    */
>> +   uint32_t tiling = I915_TILING_X;
>>     int cpp;
>>     unsigned long pitch;
>>
>> @@ -541,15 +571,17 @@ intel_create_image_common(__DRIscreen *dri_screen,
>>     assert(!(use && count));
>>
>>     uint64_t modifier = select_best_modifier(&screen->devinfo, modifiers,
>> count);
>> -   assert(modifier == DRM_FORMAT_MOD_INVALID);
>> -
>> -   if (modifier == DRM_FORMAT_MOD_INVALID && modifiers)
>> -      return NULL;
>> +   switch (modifier) {
>> +   case DRM_FORMAT_MOD_LINEAR:
>> +      tiling = I915_TILING_NONE;
>> +      break;
>> +   case DRM_FORMAT_MOD_INVALID:
>> +      if (modifiers)
>> +         return NULL;
>> +   default:
>> +         break;
>> +   }
>>
>> -   /* Historically, X-tiled was the default, and so lack of modifier means
>> -    * X-tiled.
>> -    */
>> -   tiling = I915_TILING_X;
>>     if (use & __DRI_IMAGE_USE_CURSOR) {
>>        if (width != 64 || height != 64)
>>          return NULL;
>> @@ -574,6 +606,7 @@ intel_create_image_common(__DRIscreen *dri_screen,
>>     image->width = width;
>>     image->height = height;
>>     image->pitch = pitch;
>> +   image->modifier = modifier;
>>
>>     return image;
>>  }
>> --
>> 2.12.0
>>
>>