Re: [PATCH 4/4] drm/etnaviv: Don't break exclusive fence ordering

["Christian König" <christian.koenig@amd.com>]

Am 04.04.22 um 15:14 schrieb Daniel Vetter:
> On Thu, 31 Mar 2022 at 22:46, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> There's only one exclusive slot, and we must not break the ordering.
>> Adding a new exclusive fence drops all previous fences from the
>> dma_resv. To avoid violating the signalling order we err on the side of
>> over-synchronizing by waiting for the existing fences, even if
>> userspace asked us to ignore them.
>>
>> A better fix would be to us a dma_fence_chain or _array like e.g.
>> amdgpu now uses, but it probably makes sense to lift this into
>> dma-resv.c code as a proper concept, so that drivers don't have to
>> hack up their own solution each on their own. Hence go with the simple
>> fix for now.
>>
>> Another option is the fence import ioctl from Jason:
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210610210925.642582-7-jason%40jlekstrand.net%2F&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C7db3184856cd4b6fa2ce08da163d2543%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637846749141237874%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=CQqU37VcltOuwmDN3068yv1c%2FKJ9gaf1U3T7eBQohK4%3D&amp;reserved=0
>>
>> v2: Improve commit message per Lucas' suggestion.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Russell King <linux+etnaviv@armlinux.org.uk>
>> Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
>> Cc: etnaviv@lists.freedesktop.org
> Hm thinking about this some more, with Christian's dma_resv_usage work
> this shouldn't be needed anymore.
>
> Christian, do you want me to drop this?

If it isn't committed yet we could as well just drop it. I've already 
pushed the patch which makes this superfluous.

Christian.

> -Daniel
>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> index 5f502c49aec2..14c5ff155336 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> @@ -178,19 +178,21 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit)
>>          for (i = 0; i < submit->nr_bos; i++) {
>>                  struct etnaviv_gem_submit_bo *bo = &submit->bos[i];
>>                  struct dma_resv *robj = bo->obj->base.resv;
>> +               bool write = bo->flags & ETNA_SUBMIT_BO_WRITE;
>>
>> -               if (!(bo->flags & ETNA_SUBMIT_BO_WRITE)) {
>> +               if (!(write)) {
>>                          ret = dma_resv_reserve_shared(robj, 1);
>>                          if (ret)
>>                                  return ret;
>>                  }
>>
>> -               if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT)
>> +               /* exclusive fences must be ordered */
>> +               if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT && !write)
>>                          continue;
>>
>>                  ret = drm_sched_job_add_implicit_dependencies(&submit->sched_job,
>>                                                                &bo->obj->base,
>> -                                                             bo->flags & ETNA_SUBMIT_BO_WRITE);
>> +                                                             write);
>>                  if (ret)
>>                          return ret;
>>          }
>> --
>> 2.34.1
>>
>