On 4/4/2025 9:08 PM, Ville Syrjälä wrote:
On Fri, Apr 04, 2025 at 01:35:40PM +0530, Ankit Nautiyal wrote:
Add the missing vrr parameters in vrr_params_changed() helper. This ensures that changes in vrr.vsync_{start,end} trigger a call to appropriate helpers to update the VRR registers.
Fixes: e8cd188e91bb ("drm/i915/display: Compute vrr_vsync params") Cc: Mitul Golani mitulkumar.ajitkumar.golani@intel.com Cc: Arun R Murthy arun.r.murthy@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: stable@vger.kernel.org # v6.10+ Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
drivers/gpu/drm/i915/display/intel_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index c540e2cae1f0..ced8ba0f8d6d 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -969,7 +969,9 @@ static bool vrr_params_changed(const struct intel_crtc_state *old_crtc_state, old_crtc_state->vrr.vmin != new_crtc_state->vrr.vmin || old_crtc_state->vrr.vmax != new_crtc_state->vrr.vmax || old_crtc_state->vrr.guardband != new_crtc_state->vrr.guardband ||
old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full;
old_crtc_state->vrr.pipeline_full != new_crtc_state->vrr.pipeline_full ||old_crtc_state->vrr.vsync_start != new_crtc_state->vrr.vsync_start ||old_crtc_state->vrr.vsync_end != new_crtc_state->vrr.vsync_end;These seem to yet another set of values that are potentially problematic for the always_use_vrr_tg()==true case. I'm not sure how careful we should be with this stuff...
And the whole encoder->update_pipe()/infoframe fastset stuff needs to be rewritten to make sure the all the changes it does are either atomic or happen on the correct side of the actual commit. Right now, for AS SDP specifically, we enable/disable the infoframe potentially before the actual commit, which I think is wrong at least for the disable case.
Also we still seem to be missing EMP_AS_SDP_TL completely.
Anyways, this patch isn't wrong at least so Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Thanks for the review, the patch was pushed to drm-intel-next, missed to update earlier.
Regards,
Ankit
} static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state, -- 2.45.2