| Message ID | 20201022222709.29386-10-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VRR/Adaptive Sync enabling in i915 | expand |
On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > This patch disables the VRR enable and VRR PUSH > bits in the HW during commit modeset disable sequence. > > Thsi disable will happen when the port is disabled > or when the userspace sets VRR prop to false and > requests to disable VRR. > > 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_ddi.c | 2 ++ > drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > 3 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 391c51979334..565155af3fb9 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, > > intel_disable_pipe(old_crtc_state); > > + intel_vrr_disable(old_crtc_state); > + > intel_ddi_disable_transcoder_func(old_crtc_state); > > intel_dsc_disable(old_crtc_state); > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index ec1ce88e869c..5075ecb9b5a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state) > pipe_name(pipe)); > } > > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > +{ > + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); Haven't commented on all patches, but please use i915 instead of dev_priv for new code, throughout. > + enum pipe pipe = crtc->pipe; > + u32 trans_vrr_ctl = 0, trans_push = 0; Unnecessary initializations, and in fact unnecessary variables with intel_de_rmw. > + > + if (!old_crtc_state->vrr.enable) > + return; > + > + trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe)); > + trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE); > + intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl); > + > + trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe)); > + trans_push &= ~TRANS_PUSH_EN; > + intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push); Please use intel_de_rmw for both. > + > + drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n", > + pipe_name(pipe)); drm_dbg_kms, "pipe %c" is the convention. > +} > + > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index a6b78e1676cb..8c6fd2d1bee5 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp, > 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); > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state); > > #endif /* __INTEL_VRR_H__ */
On Tue, Nov 10, 2020 at 01:01:09PM +0200, Jani Nikula wrote: > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > > This patch disables the VRR enable and VRR PUSH > > bits in the HW during commit modeset disable sequence. > > > > Thsi disable will happen when the port is disabled > > or when the userspace sets VRR prop to false and > > requests to disable VRR. > > > > 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_ddi.c | 2 ++ > > drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > > 3 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 391c51979334..565155af3fb9 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, > > > > intel_disable_pipe(old_crtc_state); > > > > + intel_vrr_disable(old_crtc_state); > > + > > intel_ddi_disable_transcoder_func(old_crtc_state); > > > > intel_dsc_disable(old_crtc_state); > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > > index ec1ce88e869c..5075ecb9b5a7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state) > > pipe_name(pipe)); > > } > > > > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > > +{ > > + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > Haven't commented on all patches, but please use i915 instead of > dev_priv for new code, throughout. > > > + enum pipe pipe = crtc->pipe; > > + u32 trans_vrr_ctl = 0, trans_push = 0; > > Unnecessary initializations, and in fact unnecessary variables with > intel_de_rmw. > Okay yes will try using the intel_de_rmw here and use the (VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE) directly in the clear field > > + > > + if (!old_crtc_state->vrr.enable) > > + return; > > + > > + trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe)); > > + trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE); > > + intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl); > > + > > + trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe)); > > + trans_push &= ~TRANS_PUSH_EN; > > + intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push); > > Please use intel_de_rmw for both. > > > + > > + drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n", > > + pipe_name(pipe)); > > drm_dbg_kms, "pipe %c" is the convention. Okay will correct it Thanks for the above feedback I will fix them in the next rev Manasi > > > +} > > + > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > > index a6b78e1676cb..8c6fd2d1bee5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp, > > 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); > > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state); > > > > #endif /* __INTEL_VRR_H__ */ > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 391c51979334..565155af3fb9 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, intel_disable_pipe(old_crtc_state); + intel_vrr_disable(old_crtc_state); + intel_ddi_disable_transcoder_func(old_crtc_state); intel_dsc_disable(old_crtc_state); diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index ec1ce88e869c..5075ecb9b5a7 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state) pipe_name(pipe)); } +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum pipe pipe = crtc->pipe; + u32 trans_vrr_ctl = 0, trans_push = 0; + + if (!old_crtc_state->vrr.enable) + return; + + trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe)); + trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE); + intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl); + + trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe)); + trans_push &= ~TRANS_PUSH_EN; + intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push); + + drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n", + pipe_name(pipe)); +} + diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index a6b78e1676cb..8c6fd2d1bee5 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp, 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); +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state); #endif /* __INTEL_VRR_H__ */
This patch disables the VRR enable and VRR PUSH bits in the HW during commit modeset disable sequence. Thsi disable will happen when the port is disabled or when the userspace sets VRR prop to false and requests to disable VRR. 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_ddi.c | 2 ++ drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_vrr.h | 1 + 3 files changed, 25 insertions(+)