Re: [PATCH v10 5/8] drm/i915: Add dedicated plane hook for async flip case

[Karthik B S <karthik.b.s@xxxxxxxxx>]

On 9/18/2020 5:23 PM, Ville Syrjälä wrote:
> On Fri, Sep 18, 2020 at 12:30:45PM +0530, Karthik B S wrote:
> > This hook is added to avoid writing other plane registers in case of
> > async flips, so that we do not write the double buffered registers
> > during async surface address update.
> >
> > v7: -Plane ctl needs bits from skl_plane_ctl_crtc as well. (Ville)
> >      -Add a vfunc for skl_program_async_surface_address
> >       and call it from intel_update_plane. (Ville)
> >
> > v8: -Rebased.
> >
> > v9: -Use if-else instead of return in intel_update_plane(). (Ville)
> >      -Rename 'program_async_surface_address' to 'async_flip'. (Ville)
> >
> > v10: -Check if async_flip hook is present before calling it.
> >        Otherwise it will OOPS during legacy cursor updates. (Ville)
> >
> > Signed-off-by: Karthik B S <karthik.b.s@xxxxxxxxx>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> > ---
> >   .../gpu/drm/i915/display/intel_atomic_plane.c |  6 ++++-
> >   .../drm/i915/display/intel_display_types.h    |  3 +++
> >   drivers/gpu/drm/i915/display/intel_sprite.c   | 24 +++++++++++++++++++
> >   3 files changed, 32 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 79032701873a..6bd8e6cdd477 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -408,7 +408,11 @@ void intel_update_plane(struct intel_plane *plane,
> >   	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >   
> >   	trace_intel_update_plane(&plane->base, crtc);
> > -	plane->update_plane(plane, crtc_state, plane_state);
> > +
> > +	if (crtc_state->uapi.async_flip && plane->async_flip)
> > +		plane->async_flip(plane, crtc_state, plane_state);
> > +	else
> > +		plane->update_plane(plane, crtc_state, plane_state);
> >   }
> >   
> >   void intel_disable_plane(struct intel_plane *plane,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 3d4bf9b6a0a2..e3339e41ddf7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1183,6 +1183,9 @@ struct intel_plane {
> >   			   struct intel_plane_state *plane_state);
> >   	int (*min_cdclk)(const struct intel_crtc_state *crtc_state,
> >   			 const struct intel_plane_state *plane_state);
> > +	void (*async_flip)(struct intel_plane *plane,
> > +			   const struct intel_crtc_state *crtc_state,
> > +			   const struct intel_plane_state *plane_state);
> >   };
> >   
> >   struct intel_watermark_params {
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 76a3d9bfe0de..3634e98b04c1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -609,6 +609,29 @@ icl_program_input_csc(struct intel_plane *plane,
> >   			  PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
> >   }
> >   
> > +static void
> > +skl_program_async_surface_address(struct intel_plane *plane,
> > +				  const struct intel_crtc_state *crtc_state,
> > +				  const struct intel_plane_state *plane_state)
>
> Pls rename this to skl_plane_async_flip() as well.

Thanks for the review.
Sure, I'll update this.
> With that
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Thanks,
Karthik.B.S
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +	unsigned long irqflags;
> > +	enum plane_id plane_id = plane->id;
> > +	enum pipe pipe = plane->pipe;
> > +	u32 surf_addr = plane_state->color_plane[0].offset;
> > +	u32 plane_ctl = plane_state->ctl;
> > +
> > +	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
> > +
> > +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > +
> > +	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
> > +	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
> > +			  intel_plane_ggtt_offset(plane_state) + surf_addr);
> > +
> > +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > +}
> > +
> >   static void
> >   skl_program_plane(struct intel_plane *plane,
> >   		  const struct intel_crtc_state *crtc_state,
> > @@ -3095,6 +3118,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> >   	plane->get_hw_state = skl_plane_get_hw_state;
> >   	plane->check_plane = skl_plane_check;
> >   	plane->min_cdclk = skl_plane_min_cdclk;
> > +	plane->async_flip = skl_program_async_surface_address;
> >   
> >   	if (INTEL_GEN(dev_priv) >= 11)
> >   		formats = icl_get_plane_formats(dev_priv, pipe,
> > --
> > 2.22.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel