[Intel-xe] [PATCH] drm/xe/display: Append dma_buf fences to framebuffer fences

[Souza, Jose]

On Fri, 2023-06-16 at 17:29 +0300, Ville Syrjälä wrote:
> On Thu, Jun 15, 2023 at 11:10:55AM +0000, Shankar, Uma wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of José Roberto
> > > de Souza
> > > Sent: Wednesday, June 7, 2023 1:50 AM
> > > To: intel-xe at lists.freedesktop.org
> > > Subject: [Intel-xe] [PATCH] drm/xe/display: Append dma_buf fences to framebuffer
> > > fences
> > > 
> > > This is required to comply with KMS page flip requirements, and it is the only way to
> > > synchronize framebuffer scanouts in Xe KMD.
> > > 
> > > UMDs are expected to attach a syncobj that will be signaled at the end of the GPU
> > > job that is rendering in the framebuffer.
> > > This way, the compositor can send jobs to the GPU to render the framebuffer and
> > > without having to wait for it to complete, queue the page flip in KMS.
> > > 
> > > This fixes display glitches.
> > 
> > Looks Good to me.
> > Reviewed-by: Uma Shankar <uma.shankar at intel.com>
> > 
> > @Ville: Can you also check this once and confirm.
> 
> Needs an actual explanation why the current code doesn't work.

Ops missed this before pushing it.
But current code don't attach dma buf fences to drm_plane_state->fence.

Drivers that don't set prepare_fb() hook have drm_gem_plane_helper_prepare_fb() called by default.
But i915 implements is own version of prepare_fb() to pin the FB, i915 if-def block does the pinning and i915 implict synchronization, while Xe if-def
block was only doing the pinning.

Again sorry for pushing this before notice that you have replied.

> 
> > 
> > Regards,
> > Uma Shankar
> > 
> > > Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8923
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > index b756561f08bd1..a3f30a319e4c0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -32,6 +32,7 @@
> > >   */
> > > 
> > >  #include <drm/drm_atomic_helper.h>
> > > +#include <drm/drm_gem_atomic_helper.h>
> > >  #include <drm/drm_blend.h>
> > >  #include <drm/drm_fourcc.h>
> > > 
> > > @@ -1120,10 +1121,20 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
> > > 
> > >  	return ret;
> > >  #else
> > > +	int ret;
> > > +
> > >  	if (!intel_fb_obj(new_plane_state->hw.fb))
> > >  		return 0;
> > > 
> > > -	return intel_plane_pin_fb(new_plane_state);
> > > +	ret = intel_plane_pin_fb(new_plane_state);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = drm_gem_plane_helper_prepare_fb(_plane, _new_plane_state);
> > > +	if (ret)
> > > +		intel_plane_unpin_fb(new_plane_state);
> > > +
> > > +	return ret;
> > >  #endif
> > >  }
> > > 
> > > --
> > > 2.41.0
> > 
>