| Message ID | 20250318073540.2773890-3-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
On Tue, Mar 18, 2025 at 01:05:26PM +0530, Ankit Nautiyal wrote: > Currently, vrr.enable is intended only for variable refresh rate timings. > At this point, we do not set fixed refresh rate timings, but the GOP can, > which creates a problem during the readback of vrr.enable. > > The GOP enables the VRR timing generator with fixed timings, while the > driver only recognizes the VRR timing generator as enabled with > variable timings. This discrepancy causes an issue due to the > fixed refresh rate check during readback. Since the VRR timing generator > is enabled and we do not support fixed timings, the readback should set > vrr.enable so that the driver can disable the VRR timing generator. > However, the current check does not allow this. > > Therefore, remove the fixed refresh rate check during readback. > > Fixes: 27217f9d1856 ("drm/i915/vrr: Track vrr.enable only for variable timing") > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index aa65a6933ddb..6bdcdfed4b9b 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -657,8 +657,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) > } > } > > - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE && > - !intel_vrr_is_fixed_rr(crtc_state); > + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; Doesn't this break the state checker when we use the VRR timing generator for fixed refresh modes? > > /* > * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags. > -- > 2.45.2
On 3/21/2025 11:04 PM, Ville Syrjälä wrote: > On Tue, Mar 18, 2025 at 01:05:26PM +0530, Ankit Nautiyal wrote: >> Currently, vrr.enable is intended only for variable refresh rate timings. >> At this point, we do not set fixed refresh rate timings, but the GOP can, >> which creates a problem during the readback of vrr.enable. >> >> The GOP enables the VRR timing generator with fixed timings, while the >> driver only recognizes the VRR timing generator as enabled with >> variable timings. This discrepancy causes an issue due to the >> fixed refresh rate check during readback. Since the VRR timing generator >> is enabled and we do not support fixed timings, the readback should set >> vrr.enable so that the driver can disable the VRR timing generator. >> However, the current check does not allow this. >> >> Therefore, remove the fixed refresh rate check during readback. >> >> Fixes: 27217f9d1856 ("drm/i915/vrr: Track vrr.enable only for variable timing") >> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_vrr.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >> index aa65a6933ddb..6bdcdfed4b9b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -657,8 +657,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) >> } >> } >> >> - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE && >> - !intel_vrr_is_fixed_rr(crtc_state); >> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; > Doesn't this break the state checker when we use the VRR timing > generator for fixed refresh modes? Yes right, but currently those changes are not merged yet. I have something like this [1], when we add support for VRR TG with fixed refresh rate. [1] https://patchwork.freedesktop.org/patch/643470/?series=134383&rev=16 Regards, Ankit > >> >> /* >> * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags. >> -- >> 2.45.2
On Fri, Mar 21, 2025 at 11:10:05PM +0530, Nautiyal, Ankit K wrote: > > On 3/21/2025 11:04 PM, Ville Syrjälä wrote: > > On Tue, Mar 18, 2025 at 01:05:26PM +0530, Ankit Nautiyal wrote: > >> Currently, vrr.enable is intended only for variable refresh rate timings. > >> At this point, we do not set fixed refresh rate timings, but the GOP can, > >> which creates a problem during the readback of vrr.enable. > >> > >> The GOP enables the VRR timing generator with fixed timings, while the > >> driver only recognizes the VRR timing generator as enabled with > >> variable timings. This discrepancy causes an issue due to the > >> fixed refresh rate check during readback. Since the VRR timing generator > >> is enabled and we do not support fixed timings, the readback should set > >> vrr.enable so that the driver can disable the VRR timing generator. > >> However, the current check does not allow this. > >> > >> Therefore, remove the fixed refresh rate check during readback. > >> > >> Fixes: 27217f9d1856 ("drm/i915/vrr: Track vrr.enable only for variable timing") > >> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_vrr.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > >> index aa65a6933ddb..6bdcdfed4b9b 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c > >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > >> @@ -657,8 +657,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) > >> } > >> } > >> > >> - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE && > >> - !intel_vrr_is_fixed_rr(crtc_state); > >> + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; > > Doesn't this break the state checker when we use the VRR timing > > generator for fixed refresh modes? > > Yes right, but currently those changes are not merged yet. > > I have something like this [1], when we add support for VRR TG with > fixed refresh rate. > > [1] https://patchwork.freedesktop.org/patch/643470/?series=134383&rev=16 Yeah, just saw it. Given that, this is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Regards, > > Ankit > > > > > >> > >> /* > >> * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags. > >> -- > >> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index aa65a6933ddb..6bdcdfed4b9b 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -657,8 +657,7 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) } } - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE && - !intel_vrr_is_fixed_rr(crtc_state); + crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; /* * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags.
Currently, vrr.enable is intended only for variable refresh rate timings. At this point, we do not set fixed refresh rate timings, but the GOP can, which creates a problem during the readback of vrr.enable. The GOP enables the VRR timing generator with fixed timings, while the driver only recognizes the VRR timing generator as enabled with variable timings. This discrepancy causes an issue due to the fixed refresh rate check during readback. Since the VRR timing generator is enabled and we do not support fixed timings, the readback should set vrr.enable so that the driver can disable the VRR timing generator. However, the current check does not allow this. Therefore, remove the fixed refresh rate check during readback. Fixes: 27217f9d1856 ("drm/i915/vrr: Track vrr.enable only for variable timing") Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_vrr.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)