Re: [Intel-gfx] [V13 5/5] drm/i915/dsi: Enable software vblank counter

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

On Wed, Sep 23, 2020 at 10:16:05AM +0000, Kulkarni, Vandita wrote:
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Wednesday, September 23, 2020 3:30 PM
> > To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
> > Subject: Re: [V13 5/5] drm/i915/dsi: Enable software vblank counter
> > 
> > On Tue, Sep 22, 2020 at 07:14:26PM +0530, Vandita Kulkarni wrote:
> > > In case of DSI cmd mode, we get hw vblank counter updated after the TE
> > > comes in, if we try to read the hw vblank counter in te handler we
> > > wouldnt have the udpated vblank counter yet.
> > > This will lead to a state where we would send the vblank event to the
> > > user space in the next te, though the frame update would have
> > > completed in the first TE duration itself.
> > > Hence switch to using software timestamp based vblank counter.
> > >
> > > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++++++
> > >  drivers/gpu/drm/i915/i915_irq.c              |  4 ++++
> > >  2 files changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index c4f331f2af45..8b9e59e52708 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1808,6 +1808,17 @@ enum pipe intel_crtc_pch_transcoder(struct
> > > intel_crtc *crtc)  static u32 intel_crtc_max_vblank_count(const struct
> > > intel_crtc_state *crtc_state)  {
> > >  	struct drm_i915_private *dev_priv =
> > > to_i915(crtc_state->uapi.crtc->dev);
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	u32 flags = crtc->mode_flags;
> > 
> > That's wrong. You need to look at the crtc_state instead.
> 
> Thanks,
> I will use crtc_state.

I'd also frop the 'flags' variable. Single use so not much point.
Or at the very least call it 'mode_flags' so we know what it
actually is.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx