Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/fbc: Moved fence related code away from intel_fbc

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Tue, 2023-08-01 at 09:42 +0200, Nirmoy Das wrote:
> 
> On 6/14/2023 7:17 AM, Jouni Högander wrote:
> > As a preparation for Xe move HW fence details away from intel_fbc
> > code.
> > Add new functions to check support for legacy fencing and fence id
> > and use
> > these in fbc code. Xe doesn't support legacy fencing.
> > 
> > v2: Fix intel_gt_support_legacy_fencing macro
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_fbc.c | 12 ++++++------
> >   drivers/gpu/drm/i915/gt/intel_gt_types.h |  2 ++
> >   drivers/gpu/drm/i915/i915_vma.h          |  5 +++++
> >   3 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index da8166eedf93..25382022cd27 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -47,6 +47,7 @@
> >   #include "i915_reg.h"
> >   #include "i915_utils.h"
> >   #include "i915_vgpu.h"
> > +#include "i915_vma.h"
> >   #include "intel_cdclk.h"
> >   #include "intel_de.h"
> >   #include "intel_display_trace.h"
> > @@ -607,7 +608,7 @@ static void ivb_fbc_activate(struct intel_fbc
> > *fbc)
> >         else if (DISPLAY_VER(i915) == 9)
> >                 skl_fbc_program_cfb_stride(fbc);
> >   
> > -       if (to_gt(i915)->ggtt->num_fences)
> > +       if (intel_gt_support_legacy_fencing(to_gt(i915)))
> >                 snb_fbc_program_fence(fbc);
> >   
> >         intel_de_write(i915, ILK_DPFC_CONTROL(fbc->id),
> > @@ -991,11 +992,10 @@ static void intel_fbc_update_state(struct
> > intel_atomic_state *state,
> >         fbc_state->fence_y_offset =
> > intel_plane_fence_y_offset(plane_state);
> >   
> >         drm_WARN_ON(&i915->drm, plane_state->flags &
> > PLANE_HAS_FENCE &&
> > -                   !plane_state->ggtt_vma->fence);
> > +                   !intel_gt_support_legacy_fencing(to_gt(i915)));
> >   
> > -       if (plane_state->flags & PLANE_HAS_FENCE &&
> > -           plane_state->ggtt_vma->fence)
> > -               fbc_state->fence_id = plane_state->ggtt_vma->fence-
> > >id;
> > +       if (plane_state->flags & PLANE_HAS_FENCE)
> > +               fbc_state->fence_id = 
> > i915_vma_fence_id(plane_state->ggtt_vma);
> >         else
> >                 fbc_state->fence_id = -1;
> >   
> > @@ -1022,7 +1022,7 @@ static bool intel_fbc_is_fence_ok(const
> > struct intel_plane_state *plane_state)
> >          */
> >         return DISPLAY_VER(i915) >= 9 ||
> >                 (plane_state->flags & PLANE_HAS_FENCE &&
> > -                plane_state->ggtt_vma->fence);
> > +                i915_vma_fence_id(plane_state->ggtt_vma) != -1);
> >   }
> >   
> >   static bool intel_fbc_is_cfb_ok(const struct intel_plane_state
> > *plane_state)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > index f08c2556aa25..121a53f908d1 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > @@ -306,4 +306,6 @@ enum intel_gt_scratch_field {
> >         INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA = 256,
> >   };
> >   
> > +#define intel_gt_support_legacy_fencing(gt) (gt->ggtt->num_fences
> > > 0)
> 
> With the checkpath warning fixed here, this is
> 
> Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>

Thank Nirmoy and Jeevan for your reviews. Whole set now merged. Also
warning mentioned above fixed before merging.

BR.

Jouni Högander

> 
> > +
> >   #endif /* __INTEL_GT_TYPES_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_vma.h
> > b/drivers/gpu/drm/i915/i915_vma.h
> > index 9a9729205d5b..6fdf6205c290 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.h
> > +++ b/drivers/gpu/drm/i915/i915_vma.h
> > @@ -418,6 +418,11 @@ i915_vma_unpin_fence(struct i915_vma *vma)
> >                 __i915_vma_unpin_fence(vma);
> >   }
> >   
> > +static inline int i915_vma_fence_id(const struct i915_vma *vma)
> > +{
> > +       return vma->fence ? vma->fence->id : -1;
> > +}
> > +
> >   void i915_vma_parked(struct intel_gt *gt);
> >   
> >   static inline bool i915_vma_is_scanout(const struct i915_vma
> > *vma)