| Message ID | 20200922134426.9840-6-vandita.kulkarni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for mipi dsi cmd mode | expand |
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. > + > + /* > + * From Gen 11, In case of dsi cmd mode, frame counter wouldnt > + * have updated at the beginning of TE, if we want to use > + * the hw counter, then we would find it updated in only > + * the next TE, hence switching to sw counter. > + */ > + if (flags & (I915_MODE_FLAG_DSI_USE_TE0 | I915_MODE_FLAG_DSI_USE_TE1)) > + return 0; > > /* > * On i965gm the hardware frame counter reads > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index c2e4b227bdf3..634c60befe7e 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -682,8 +682,12 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc) > u32 g4x_get_vblank_counter(struct drm_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > + struct drm_vblank_crtc *vblank = &dev_priv->drm.vblank[drm_crtc_index(crtc)]; > enum pipe pipe = to_intel_crtc(crtc)->pipe; > > + if (!vblank->max_vblank_count) > + return 0; > + > return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); > } > > -- > 2.21.0.5.gaeb582a
> -----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 saw this happening crtc->mode_flags = crtc_state->mode_flags; at intel_crtc_update_active_timings. Thanks, -Vandita > > > + > > + /* > > + * From Gen 11, In case of dsi cmd mode, frame counter wouldnt > > + * have updated at the beginning of TE, if we want to use > > + * the hw counter, then we would find it updated in only > > + * the next TE, hence switching to sw counter. > > + */ > > + if (flags & (I915_MODE_FLAG_DSI_USE_TE0 | > I915_MODE_FLAG_DSI_USE_TE1)) > > + return 0; > > > > /* > > * On i965gm the hardware frame counter reads diff --git > > a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index c2e4b227bdf3..634c60befe7e 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -682,8 +682,12 @@ u32 i915_get_vblank_counter(struct drm_crtc > > *crtc) > > u32 g4x_get_vblank_counter(struct drm_crtc *crtc) { > > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > > + struct drm_vblank_crtc *vblank = > > +&dev_priv->drm.vblank[drm_crtc_index(crtc)]; > > enum pipe pipe = to_intel_crtc(crtc)->pipe; > > > > + if (!vblank->max_vblank_count) > > + return 0; > > + > > return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); > > } > > > > -- > > 2.21.0.5.gaeb582a > > -- > Ville Syrjälä > Intel
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.
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, September 23, 2020 4:05 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 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. Ok, will use mode_flags. Thanks, Vandita > > -- > Ville Syrjälä > Intel
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; + + /* + * From Gen 11, In case of dsi cmd mode, frame counter wouldnt + * have updated at the beginning of TE, if we want to use + * the hw counter, then we would find it updated in only + * the next TE, hence switching to sw counter. + */ + if (flags & (I915_MODE_FLAG_DSI_USE_TE0 | I915_MODE_FLAG_DSI_USE_TE1)) + return 0; /* * On i965gm the hardware frame counter reads diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c2e4b227bdf3..634c60befe7e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -682,8 +682,12 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc) u32 g4x_get_vblank_counter(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); + struct drm_vblank_crtc *vblank = &dev_priv->drm.vblank[drm_crtc_index(crtc)]; enum pipe pipe = to_intel_crtc(crtc)->pipe; + if (!vblank->max_vblank_count) + return 0; + return I915_READ(PIPE_FRMCOUNT_G4X(pipe)); }
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(+)