[Mesa-dev] [Patch] Sharing flags should disable tiling

davyaxel at free.fr davyaxel at free.fr
Mon Aug 12 14:54:21 PDT 2013


> On 08/11/2013 03:50 AM, davyaxel at free.fr wrote:
>>> This looks good to me, but commit messages should have a prefix
>>> indicating which component is changed. In your case it's "gallium:"
>>> and "intel:", respectively.
>>>
>>> Marek 
>>
>>
>> Ok, I've suppressed some trailing spaces and changed the commit message. 
>
> And smashed two separate patches into one.  Was that intentional? 
I thought it would be more simple if it was only one patch.
>>  From f48fdb44d638ae850c7f3df36211b33788088927 Mon Sep 17 00:00:00 2001
>> From: axeldavy <axel.davy at ens.fr>
>> Date: Sun, 11 Aug 2013 12:36:33 +0200
>> Subject: [PATCH 1/1] gallium: Implements the new meaning of PIPE_BIND_SHARED
>>   (there should be no tiling) for i915, ilo, nv50, nvc0, r300, r600, radeonsi.
>>   intel: _DRI_IMAGE_USE_SHARE is equivalent to PIPE_BIND_SHARED. No tiling
>>   either for the dri drivers i915 and i965.
>>
>>
>> Signed-off-by: axeldavy <axel.davy at ens.fr> 
>
> Real name in S-o-b, please.  Axel Davy?
Yes my name is Axel Davy
>> ---
>>   src/gallium/drivers/i915/i915_resource.c    | 7 ++++++-
>>   src/gallium/drivers/ilo/ilo_resource.c      | 2 +-
>>   src/gallium/drivers/nv50/nv50_miptree.c     | 3 +++
>>   src/gallium/drivers/nvc0/nvc0_miptree.c     | 3 +++
>>   src/gallium/drivers/r300/r300_texture.c     | 2 +-
>>   src/gallium/drivers/r600/r600_texture.c     | 3 ++-
>>   src/gallium/drivers/radeonsi/r600_texture.c | 2 +-
>>   src/gallium/include/pipe/p_defines.h        | 5 ++---
>>   src/mesa/drivers/dri/i915/intel_screen.c    | 2 ++
>>   src/mesa/drivers/dri/i965/intel_screen.c    | 3 ++-
>>   10 files changed, 23 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/gallium/drivers/i915/i915_resource.c b/src/gallium/drivers/i915/i915_resource.c
>> index 314ebe9..563bf83 100644
>> --- a/src/gallium/drivers/i915/i915_resource.c
>> +++ b/src/gallium/drivers/i915/i915_resource.c
>> @@ -12,7 +12,12 @@ i915_resource_create(struct pipe_screen *screen,
>>      if (template->target == PIPE_BUFFER)
>>         return i915_buffer_create(screen, template);
>>      else
>> -      return i915_texture_create(screen, template, FALSE);
>> +   {
>> +      if (!(template->bind & PIPE_BIND_SHARED))
>> +         return i915_texture_create(screen, template, FALSE);
>> +      else
>> +         return i915_texture_create(screen, template, TRUE);
>> +   }
>>
>>   }
>>
>> diff --git a/src/gallium/drivers/ilo/ilo_resource.c b/src/gallium/drivers/ilo/ilo_resource.c
>> index 5061f69..3d5874f 100644
>> --- a/src/gallium/drivers/ilo/ilo_resource.c
>> +++ b/src/gallium/drivers/ilo/ilo_resource.c
>> @@ -473,7 +473,7 @@ tex_layout_init_tiling(struct tex_layout *layout)
>>       *     "The cursor surface address must be 4K byte aligned. The cursor must
>>       *      be in linear memory, it cannot be tiled."
>>       */
>> -   if (unlikely(templ->bind & PIPE_BIND_CURSOR))
>> +   if (unlikely(templ->bind & (PIPE_BIND_CURSOR | PIPE_BIND_SHARED)))
>>         valid_tilings &= tile_none;
>>
>>      /*
>> diff --git a/src/gallium/drivers/nv50/nv50_miptree.c b/src/gallium/drivers/nv50/nv50_miptree.c
>> index 28be768..a4f15fe 100644
>> --- a/src/gallium/drivers/nv50/nv50_miptree.c
>> +++ b/src/gallium/drivers/nv50/nv50_miptree.c
>> @@ -326,6 +326,9 @@ nv50_miptree_create(struct pipe_screen *pscreen,
>>      pipe_reference_init(&pt->reference, 1);
>>      pt->screen = pscreen;
>>
>> +   if (pt->bind & PIPE_BIND_SHARED)
>> +      pt->flags |= NOUVEAU_RESOURCE_FLAG_LINEAR;
>> +
>>      bo_config.nv50.memtype = nv50_mt_choose_storage_type(mt, TRUE);
>>
>>      if (!nv50_miptree_init_ms_mode(mt)) {
>> diff --git a/src/gallium/drivers/nvc0/nvc0_miptree.c b/src/gallium/drivers/nvc0/nvc0_miptree.c
>> index 9e57d74..e645553 100644
>> --- a/src/gallium/drivers/nvc0/nvc0_miptree.c
>> +++ b/src/gallium/drivers/nvc0/nvc0_miptree.c
>> @@ -274,6 +274,9 @@ nvc0_miptree_create(struct pipe_screen *pscreen,
>>         }
>>      }
>>
>> +   if (pt->bind & PIPE_BIND_SHARED)
>> +      pt->flags |= NOUVEAU_RESOURCE_FLAG_LINEAR;
>> +
>>      bo_config.nvc0.memtype = nvc0_mt_choose_storage_type(mt, compressed);
>>
>>      if (!nvc0_miptree_init_ms_mode(mt)) {
>> diff --git a/src/gallium/drivers/r300/r300_texture.c b/src/gallium/drivers/r300/r300_texture.c
>> index 13e9bc3..3672d7b 100644
>> --- a/src/gallium/drivers/r300/r300_texture.c
>> +++ b/src/gallium/drivers/r300/r300_texture.c
>> @@ -1079,7 +1079,7 @@ struct pipe_resource *r300_texture_create(struct pipe_screen *screen,
>>       enum radeon_bo_layout microtile, macrotile;
>>
>>       if ((base->flags & R300_RESOURCE_FLAG_TRANSFER) ||
>> -        (base->bind & PIPE_BIND_SCANOUT)) {
>> +        (base->bind & (PIPE_BIND_SCANOUT | PIPE_BIND_SHARED))) {
>>           microtile = RADEON_LAYOUT_LINEAR;
>>           macrotile = RADEON_LAYOUT_LINEAR;
>>       } else {
>> diff --git a/src/gallium/drivers/r600/r600_texture.c b/src/gallium/drivers/r600/r600_texture.c
>> index 36cca17..60c050b 100644
>> --- a/src/gallium/drivers/r600/r600_texture.c
>> +++ b/src/gallium/drivers/r600/r600_texture.c
>> @@ -609,7 +609,8 @@ struct pipe_resource *r600_texture_create(struct pipe_screen *screen,
>>        * because 422 formats are used for videos, which prefer linear buffers
>>        * for fast uploads anyway. */
>>       if (!(templ->flags & R600_RESOURCE_FLAG_TRANSFER) &&
>> -        desc->layout != UTIL_FORMAT_LAYOUT_SUBSAMPLED) {
>> +        (desc->layout != UTIL_FORMAT_LAYOUT_SUBSAMPLED) &&
>> +        !(templ->bind & PIPE_BIND_SHARED)) {
>>           if (templ->flags & R600_RESOURCE_FLAG_FORCE_TILING) {
>>               array_mode = V_038000_ARRAY_2D_TILED_THIN1;
>>           } else if (!(templ->bind & PIPE_BIND_SCANOUT) &&
>> diff --git a/src/gallium/drivers/radeonsi/r600_texture.c b/src/gallium/drivers/radeonsi/r600_texture.c
>> index 282d4f2..604d315 100644
>> --- a/src/gallium/drivers/radeonsi/r600_texture.c
>> +++ b/src/gallium/drivers/radeonsi/r600_texture.c
>> @@ -528,7 +528,7 @@ struct pipe_resource *si_texture_create(struct pipe_screen *screen,
>>       int r;
>>
>>       if (!(templ->flags & R600_RESOURCE_FLAG_TRANSFER) &&
>> -        !(templ->bind & PIPE_BIND_SCANOUT)) {
>> +        !(templ->bind & (PIPE_BIND_SCANOUT | PIPE_BIND_SHARED))) {
>>           if (util_format_is_compressed(templ->format)) {
>>               array_mode = V_009910_ARRAY_1D_TILED_THIN1;
>>           } else {
>> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
>> index fb42cdf..b6ea7a7 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -327,9 +327,8 @@ enum pipe_flush_flags {
>>    * implies extra layout constraints on some hardware.  It may also
>>    * have some special meaning regarding mouse cursor images.
>>    *
>> - * The shared flag is quite underspecified, but certainly isn't a
>> - * binding flag - it seems more like a message to the winsys to create
>> - * a shareable allocation.
>> + * The shared flag means that allocations must be shareable, and then until tiling
>> + * sets up automatically when needed, that there must be no tiling.
>>    */
>>   #define PIPE_BIND_SCANOUT     (1 << 14) /*  */
>>   #define PIPE_BIND_SHARED      (1 << 15) /* get_texture_handle ??? */
>> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
>> index 30a867e..263dcc4 100644
>> --- a/src/mesa/drivers/dri/i915/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i915/intel_screen.c 
>
> Are the changes to i915 necessary?  Other than hybrid systems (of which there are none with i915 graphics), is there any case where __DRI_IMAGE_USE_SHARE can occur?  It seems like these cases should either be ignored or generate an error. 
I don't know how it could be used (maybe external usb graphic card?), but I think the flag should mean the same for every drivers, that's why I made the change here too.

>> @@ -483,6 +483,8 @@ intel_create_image(__DRIscreen *screen,
>>        return NULL;
>>         tiling = I915_TILING_NONE;
>>      } 
>
> Blank line here. 
Do I need to generate a new patch with the new Blank lines? (and git send-email)

Thanks for your review.
>> +   if (use & __DRI_IMAGE_USE_SHARE)
>> +      tiling = I915_TILING_NONE;
>>
>>      image = intel_allocate_image(format, loaderPrivate);
>>      if (image == NULL)
>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
>> index 4ee8602..4caf6fe 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -505,7 +505,8 @@ intel_create_image(__DRIscreen *screen,
>>        return NULL;
>>         tiling = I915_TILING_NONE;
>>      } 
>
> Blank line here.
>> -
>> +   if (use & __DRI_IMAGE_USE_SHARE)
>> +      tiling = I915_TILING_NONE; 
>
> And here.
>>      image = intel_allocate_image(format, loaderPrivate);
>>      if (image == NULL)
>>         return NULL;
>


More information about the mesa-dev mailing list