| Message ID | 20241001134703.1128487-2-mitulkumar.ajitkumar.golani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VRR refactoring and panel replay workaround | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Mitul Golani Sent: Tuesday, October 1, 2024 6:47 AM To: intel-gfx@lists.freedesktop.org Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma <uma.shankar@intel.com> Subject: [PATCH v13 1/3] drm/i915/vrr: Add helper to check if vrr possible > > Add helper to check if vrr is possible based on flipline > is computed. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 7 ++++++- > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 9a51f5bac307..79db30e31324 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -56,6 +56,11 @@ bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh) > vrefresh <= info->monitor_range.max_vfreq; > } > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state) > +{ > + return (crtc_state->vrr.flipline) ? true : false; I think this can be compressed to: """ return !!(crtc_state->vrr.flipline); """ But otherwise: Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > +} > + > void > intel_vrr_check_modeset(struct intel_atomic_state *state) > { > @@ -281,7 +286,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > intel_de_rmw(display, CHICKEN_TRANS(cpu_transcoder), > 0, PIPE_VBLANK_WITH_DELAY); > > - if (!crtc_state->vrr.flipline) { > + if (!intel_vrr_possible(crtc_state)) { > intel_de_write(display, > TRANS_VRR_CTL(display, cpu_transcoder), 0); > return; > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 89937858200d..af921dda4619 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -15,6 +15,7 @@ struct intel_crtc_state; > > bool intel_vrr_is_capable(struct intel_connector *connector); > bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh); > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state); > void intel_vrr_check_modeset(struct intel_atomic_state *state); > void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state); > -- > 2.46.0 > >
On Tue, Oct 01, 2024 at 07:17:01PM +0530, Mitul Golani wrote: > Add helper to check if vrr is possible based on flipline > is computed. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 7 ++++++- > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 9a51f5bac307..79db30e31324 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -56,6 +56,11 @@ bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh) > vrefresh <= info->monitor_range.max_vfreq; > } > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state) > +{ > + return (crtc_state->vrr.flipline) ? true : false; That can be just 'return crtc_state->vrr.flipline;', of 'return crtc_state->vrr.flipline != 0;' if you prefer. > +} > + > void > intel_vrr_check_modeset(struct intel_atomic_state *state) > { > @@ -281,7 +286,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > intel_de_rmw(display, CHICKEN_TRANS(cpu_transcoder), > 0, PIPE_VBLANK_WITH_DELAY); > > - if (!crtc_state->vrr.flipline) { > + if (!intel_vrr_possible(crtc_state)) { Hmm. Looks like we have a fairly big mess with the AS SDP and CMRR stuff when it comes to programming the hardware vs. readout. But that's not the fault of this patch obviously. > intel_de_write(display, > TRANS_VRR_CTL(display, cpu_transcoder), 0); > return; > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 89937858200d..af921dda4619 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -15,6 +15,7 @@ struct intel_crtc_state; > > bool intel_vrr_is_capable(struct intel_connector *connector); > bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh); > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state); > void intel_vrr_check_modeset(struct intel_atomic_state *state); > void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state); > -- > 2.46.0
> -----Original Message----- > From: Cavitt, Jonathan <jonathan.cavitt@intel.com> > Sent: Tuesday, October 1, 2024 7:37 PM > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>; > intel-gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; > Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com> > Subject: RE: [PATCH v13 1/3] drm/i915/vrr: Add helper to check if vrr possible > > -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Mitul > Golani > Sent: Tuesday, October 1, 2024 6:47 AM > To: intel-gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; > Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma > <uma.shankar@intel.com> > Subject: [PATCH v13 1/3] drm/i915/vrr: Add helper to check if vrr possible > > > > Add helper to check if vrr is possible based on flipline is computed. > > > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_vrr.c | 7 ++++++- > > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > > b/drivers/gpu/drm/i915/display/intel_vrr.c > > index 9a51f5bac307..79db30e31324 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -56,6 +56,11 @@ bool intel_vrr_is_in_range(struct intel_connector > *connector, int vrefresh) > > vrefresh <= info->monitor_range.max_vfreq; } > > > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state) { > > + return (crtc_state->vrr.flipline) ? true : false; > > I think this can be compressed to: > > """ > return !!(crtc_state->vrr.flipline); > """ > > But otherwise: > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > Thanks @Cavitt, Jonathan for the review, Agreed, with suggested change. On next version will be returning just 'crtc_state->vrr.flipline' or 'return crtc_state->vrr.flipline != 0;' as suggested by ville in latest review. Again Thanks > > +} > > + > > void > > intel_vrr_check_modeset(struct intel_atomic_state *state) { @@ > > -281,7 +286,7 @@ void intel_vrr_set_transcoder_timings(const struct > intel_crtc_state *crtc_state) > > intel_de_rmw(display, CHICKEN_TRANS(cpu_transcoder), > > 0, PIPE_VBLANK_WITH_DELAY); > > > > - if (!crtc_state->vrr.flipline) { > > + if (!intel_vrr_possible(crtc_state)) { > > intel_de_write(display, > > TRANS_VRR_CTL(display, cpu_transcoder), 0); > > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h > > b/drivers/gpu/drm/i915/display/intel_vrr.h > > index 89937858200d..af921dda4619 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -15,6 +15,7 @@ struct intel_crtc_state; > > > > bool intel_vrr_is_capable(struct intel_connector *connector); bool > > intel_vrr_is_in_range(struct intel_connector *connector, int > > vrefresh); > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state); > > void intel_vrr_check_modeset(struct intel_atomic_state *state); void > > intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > > struct drm_connector_state *conn_state); > > -- > > 2.46.0 > > > >
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, October 9, 2024 12:55 PM > To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>; > Syrjala, Ville <ville.syrjala@intel.com>; Nautiyal, Ankit K > <ankit.k.nautiyal@intel.com>; Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH v13 1/3] drm/i915/vrr: Add helper to check if vrr possible > > On Tue, Oct 01, 2024 at 07:17:01PM +0530, Mitul Golani wrote: > > Add helper to check if vrr is possible based on flipline is computed. > > > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_vrr.c | 7 ++++++- > > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > > b/drivers/gpu/drm/i915/display/intel_vrr.c > > index 9a51f5bac307..79db30e31324 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -56,6 +56,11 @@ bool intel_vrr_is_in_range(struct intel_connector > *connector, int vrefresh) > > vrefresh <= info->monitor_range.max_vfreq; } > > > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state) { > > + return (crtc_state->vrr.flipline) ? true : false; > > That can be just 'return crtc_state->vrr.flipline;', of 'return crtc_state- > >vrr.flipline != 0;' if you prefer. > Thanks Ville for the review, I will update it to just return crtc_state->vrr.flipline in next revision. > > +} > > + > > void > > intel_vrr_check_modeset(struct intel_atomic_state *state) { @@ > > -281,7 +286,7 @@ void intel_vrr_set_transcoder_timings(const struct > intel_crtc_state *crtc_state) > > intel_de_rmw(display, CHICKEN_TRANS(cpu_transcoder), > > 0, PIPE_VBLANK_WITH_DELAY); > > > > - if (!crtc_state->vrr.flipline) { > > + if (!intel_vrr_possible(crtc_state)) { > > Hmm. Looks like we have a fairly big mess with the AS SDP and CMRR stuff > when it comes to programming the hardware vs. readout. But that's not the > fault of this patch obviously. > > > intel_de_write(display, > > TRANS_VRR_CTL(display, cpu_transcoder), 0); > > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h > > b/drivers/gpu/drm/i915/display/intel_vrr.h > > index 89937858200d..af921dda4619 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -15,6 +15,7 @@ struct intel_crtc_state; > > > > bool intel_vrr_is_capable(struct intel_connector *connector); bool > > intel_vrr_is_in_range(struct intel_connector *connector, int > > vrefresh); > > +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state); > > void intel_vrr_check_modeset(struct intel_atomic_state *state); void > > intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > > struct drm_connector_state *conn_state); > > -- > > 2.46.0 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 9a51f5bac307..79db30e31324 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -56,6 +56,11 @@ bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh) vrefresh <= info->monitor_range.max_vfreq; } +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state) +{ + return (crtc_state->vrr.flipline) ? true : false; +} + void intel_vrr_check_modeset(struct intel_atomic_state *state) { @@ -281,7 +286,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) intel_de_rmw(display, CHICKEN_TRANS(cpu_transcoder), 0, PIPE_VBLANK_WITH_DELAY); - if (!crtc_state->vrr.flipline) { + if (!intel_vrr_possible(crtc_state)) { intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 0); return; diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index 89937858200d..af921dda4619 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -15,6 +15,7 @@ struct intel_crtc_state; bool intel_vrr_is_capable(struct intel_connector *connector); bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh); +bool intel_vrr_possible(const struct intel_crtc_state *crtc_state); void intel_vrr_check_modeset(struct intel_atomic_state *state); void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state);
Add helper to check if vrr is possible based on flipline is computed. Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> --- drivers/gpu/drm/i915/display/intel_vrr.c | 7 ++++++- drivers/gpu/drm/i915/display/intel_vrr.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-)