[PATCH 2/5] drm/exec: don't immediately add the prelocked obj

[Thomas Hellström]

On Fri, 2024-07-05 at 14:41 +0200, Christian König wrote:
> Am 03.07.24 um 17:51 schrieb Thomas Hellström:
> > On Wed, 2024-07-03 at 15:25 +0200, Christian König wrote:
> > > Some contended objects might never be locked again in the case of
> > > eviction
> > > handling for example.
> > > 
> > > Make sure that those doesn't show up in the list of locked
> > > objects
> > > until
> > > they are explicitely mentioned.
> > Could you be a bit more specific in the commit message about in
> > what
> > situations that is bad?
> 
> The prelocked object is not necessarily expected to be in the list of
> locked objects.
> 
> I ran into issues because amdgpu tried to validate all locked objects
> and so tried to also validate the prelocked one (which was only
> locked 
> for eviction).
> 
> That obviously didn't made much sense.

Indeed. Could you add a similar description to the commit message?

/Thomas



> 
> Regards,
> Christian.
> 
> > 
> > /Thomas
> > 
> > 
> > 
> > > Signed-off-by: Christian König <christian.koenig at amd.com>
> > > ---
> > >   drivers/gpu/drm/drm_exec.c | 18 +++++++++---------
> > >   1 file changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_exec.c
> > > b/drivers/gpu/drm/drm_exec.c
> > > index 2da094bdf8a4..220df336fbd9 100644
> > > --- a/drivers/gpu/drm/drm_exec.c
> > > +++ b/drivers/gpu/drm/drm_exec.c
> > > @@ -61,8 +61,11 @@ static void drm_exec_unlock_all(struct
> > > drm_exec
> > > *exec)
> > >   		drm_gem_object_put(obj);
> > >   	}
> > >   
> > > -	drm_gem_object_put(exec->prelocked);
> > > -	exec->prelocked = NULL;
> > > +	if (exec->prelocked) {
> > > +		dma_resv_unlock(exec->prelocked->resv);
> > > +		drm_gem_object_put(exec->prelocked);
> > > +		exec->prelocked = NULL;
> > > +	}
> > >   }
> > >   
> > >   /**
> > > @@ -179,16 +182,9 @@ static int drm_exec_lock_contended(struct
> > > drm_exec *exec)
> > >   		dma_resv_lock_slow(obj->resv, &exec->ticket);
> > >   	}
> > >   
> > > -	ret = drm_exec_obj_locked(exec, obj);
> > > -	if (unlikely(ret))
> > > -		goto error_unlock;
> > > -
> > >   	exec->prelocked = obj;
> > >   	return 0;
> > >   
> > > -error_unlock:
> > > -	dma_resv_unlock(obj->resv);
> > > -
> > >   error_dropref:
> > >   	drm_gem_object_put(obj);
> > >   	return ret;
> > > @@ -214,6 +210,10 @@ int drm_exec_lock_obj(struct drm_exec *exec,
> > > struct drm_gem_object *obj)
> > >   		return ret;
> > >   
> > >   	if (exec->prelocked == obj) {
> > > +		ret = drm_exec_obj_locked(exec, obj);
> > > +		if (unlikely(ret))
> > > +			return ret;
> > > +
> > >   		drm_gem_object_put(exec->prelocked);
> > >   		exec->prelocked = NULL;
> > >   		return 0;
>