Re: [Intel-gfx] [PATCH v2] drm/i915/display: Use dma_fence interfaces instead of i915_sw_fence

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 25, 2023 at 05:18:00PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 20, 2023 at 12:41:03PM +0300, Jouni Högander wrote:
> > We are preparing for Xe driver. Xe driver doesn't have i915_sw_fence
> > implementation. Lets drop i915_sw_fence usage from display code and
> > use dma_fence interfaces directly.
> > 
> > For this purpose stack dma fences from related objects into new plane
> > state. Drm_gem_plane_helper_prepare_fb can be used for fences in new
> > fb. Separate local implementation is used for Stacking fences from old fb
> > into new plane state. Then wait for these stacked fences during atomic
> > commit. There is no be need for separate GPU reset handling in
> > intel_atomic_commit_fence_wait as the fences are signaled when GPU hang is
> > detected and GPU is being reset.
> > 
> > v2:
> >   - Add fences from old fb into new_plane_state->uapi.fence rather than
> >     into old_plane_state->uapi.fence
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_atomic.c   |  3 -
> >  .../gpu/drm/i915/display/intel_atomic_plane.c | 89 +++++++++++--------
> >  drivers/gpu/drm/i915/display/intel_display.c  | 78 ++++++----------
> >  .../drm/i915/display/intel_display_types.h    |  2 -
> >  4 files changed, 77 insertions(+), 95 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index 5d18145da279..ec0d5168b503 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -331,9 +331,6 @@ void intel_atomic_state_free(struct drm_atomic_state *_state)
> >  
> >  	drm_atomic_state_default_release(&state->base);
> >  	kfree(state->global_objs);
> > -
> > -	i915_sw_fence_fini(&state->commit_ready);
> > -
> >  	kfree(state);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index b1074350616c..20fd12df6850 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -31,7 +31,10 @@
> >   * prepare/check/commit/cleanup steps.
> >   */
> >  
> > +#include <linux/dma-fence-chain.h>
> > +
> >  #include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_gem_atomic_helper.h>
> >  #include <drm/drm_blend.h>
> >  #include <drm/drm_fourcc.h>
> >  
> > @@ -1012,6 +1015,44 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
> >  	return 0;
> >  }
> >  
> > +static int add_fences(struct dma_resv *obj,
> > +		      struct drm_plane_state *dst)
> 
> I would name things differently:
> s/obj/resv/
> s/dst/plane_state/

Probably should be "new_plane_state" to make it clear what
must be passed in.

-- 
Ville Syrjälä
Intel