Re: [Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame

From: "Navare, Manasi" <manasi.d.navare@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame
Date: Tue, 1 Dec 2020 14:57:29 -0800	[thread overview]
Message-ID: <20201201225729.GD22644@labuser-Z97X-UD5H> (raw)
In-Reply-To: <87ft5hscjl.fsf@intel.com>

On Tue, Nov 10, 2020 at 12:59:10PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > VRR achieves vblank stretching using the HW PUSH functionality.
> > So once the VRR is enabled during modeset then for each flip
> > request from userspace, in the atomic tail pipe_update_end()
> > we need to set the VRR push bit in HW for it to terminate
> > the vblank at configured flipline or anytime after flipline
> > or latest at the Vmax.
> >
> > The HW clears the PUSH bit after the double buffer updates
> > are completed.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sprite.c |  5 +++++
> >  drivers/gpu/drm/i915/display/intel_vrr.c    | 17 +++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h    |  1 +
> >  3 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index b6deeb338477..cb10fe462f06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -49,6 +49,7 @@
> >  #include "intel_psr.h"
> >  #include "intel_dsi.h"
> >  #include "intel_sprite.h"
> > +#include "intel_vrr.h"
> >  
> >  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> >  			     int usecs)
> > @@ -217,6 +218,10 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> >  	    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
> >  		icl_dsi_frame_update(new_crtc_state);
> >  
> > +	/* Send VRR Push to terminate Vblank */
> > +	if (new_crtc_state->vrr.enable)
> > +		intel_vrr_send_push(new_crtc_state);
> > +
> 
> Maybe move the vrr.enable check to the function?

Yes makes sense will do

> 
> >  	/* We're still in the vblank-evade critical section, this can't race.
> >  	 * Would be slightly nice to just grab the vblank count and arm the
> >  	 * event outside of the critical section - the spinlock might spin for a
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 7f1353bac583..ec1ce88e869c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -102,3 +102,20 @@ void intel_vrr_enable(struct intel_encoder *encoder,
> >  		trans_push);
> >  }
> >  
> > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	enum pipe pipe = crtc->pipe;
> > +	u32 trans_push;
> > +
> > +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> > +	WARN_ON(!(trans_push & TRANS_PUSH_EN));
> 
> drm_WARN_ON, and perhaps move this below the register rmw. It doesn't
> change the flow anyway.

Yes will do

Manasi


> 
> > +
> > +	trans_push |= TRANS_PUSH_SEND;
> > +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> > +
> > +	drm_dbg(&dev_priv->drm, "Sending VRR Push on Pipe (%c)\n",
> > +		pipe_name(pipe));
> 
> drm_dbg_kms
> 
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index 05d982d6fbae..a6b78e1676cb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -19,5 +19,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state);
> >  void intel_vrr_enable(struct intel_encoder *encoder,
> >  		      const struct intel_crtc_state *crtc_state);
> > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx