From: Ville Syrjälä <[email protected]>

Adjust the panel fixed mode selection algorithm to only consider
fixed modes that are "VRR compatible" with the old fixed mode
when userspace doesn't want to allow full modesets. This will
allow a VRR based refresh rate changes (ie. just a change in
the vblank length) via the fastset path.

When full modesets are allowed, we still use the original algorithm
as that may pick a fixed mode with a more optimal dotclock, potentially
leading to reduced power consumption.

This approach works as long as userspace does the initial
allow_modeset=true commit using the highest refresh rate it will
want to use. Subsequent commits with allow_modeset=false can then
switch between lower refresh rates without blinks.

One remaining hurdle we may need to solve is the guardband length.
Assuming the highest refresh rate vblank is too short for
intel_vrr_compute_optimized_guardband() the intitial guardband will
match the highest refresh rate vblank. A subsequent switch to a lower
refresh rate will then recompute the guardband and select a value
that is higher (since the vblank will be longer). The mismatch in
guardband lengths will prevent the fastset. We may either have to
preserve the original (sub-optimal) guardband, or we'll have to
revisit the idea of changing the guardband without a full modeset.

Note that I'm not 100% happy with this solution because
intel_panel_fixed_mode() is no longer fully idempotent, but I wasn't
able to come up with anything truly better either :/ The simple
solution would be just to always pick the fixed mode with the highest
dotclock, but that could lead to increased power consumption even
when high refresh rates are never used.

Perhaps the proper solution would be to just deprecate this
idea of taking in random modes for internal panels and then
cooking up a compatible fixed modes. Life would be easier if
userspace was required to provide the desired fixed mode directly.
But in order to do that we'd need to introduce new uapi properties
to control the pfit aspect of this, and we'd probably need a new
client cap to select between the old and new userspace behaviour.
Something to consider in the future...

Signed-off-by: Ville Syrjälä <[email protected]>
---
 drivers/gpu/drm/i915/display/intel_panel.c | 55 ++++++++++++++++++++--
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c 
b/drivers/gpu/drm/i915/display/intel_panel.c
index af59fc946fcb..a5fcac1318da 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -82,16 +82,37 @@ static bool is_best_fixed_mode(struct intel_connector 
*connector,
                abs(drm_mode_vrefresh(best_mode) - vrefresh);
 }
 
-const struct drm_display_mode *
-intel_panel_fixed_mode(struct intel_connector *connector,
-                      const struct drm_display_mode *mode)
+static bool is_vrr_compatible(const struct drm_display_mode *mode1,
+                             const struct drm_display_mode *mode2)
+{
+       return drm_mode_match(mode1, mode2,
+                             DRM_MODE_MATCH_CLOCK |
+                             DRM_MODE_MATCH_TIMINGS_VRR |
+                             DRM_MODE_MATCH_FLAGS |
+                             DRM_MODE_MATCH_3D_FLAGS);
+}
+
+static const struct drm_display_mode *
+_intel_panel_fixed_mode(struct intel_connector *connector,
+                       const struct drm_display_mode *mode,
+                       const struct drm_display_mode *vrr_ref_mode)
 {
        const struct drm_display_mode *fixed_mode, *best_mode = NULL;
        int vrefresh = drm_mode_vrefresh(mode);
 
+       if (vrr_ref_mode &&
+           (!intel_vrr_is_in_range(connector, vrefresh) ||
+            !intel_vrr_is_in_range(connector, 
drm_mode_vrefresh(vrr_ref_mode))))
+               return NULL;
+
        list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
                int fixed_mode_vrefresh = drm_mode_vrefresh(fixed_mode);
 
+               if (vrr_ref_mode &&
+                   (!intel_vrr_is_in_range(connector, fixed_mode_vrefresh) ||
+                    !is_vrr_compatible(fixed_mode, vrr_ref_mode)))
+                       continue;
+
                if (is_best_fixed_mode(connector, vrefresh,
                                       fixed_mode_vrefresh, best_mode))
                        best_mode = fixed_mode;
@@ -100,6 +121,13 @@ intel_panel_fixed_mode(struct intel_connector *connector,
        return best_mode;
 }
 
+const struct drm_display_mode *
+intel_panel_fixed_mode(struct intel_connector *connector,
+                      const struct drm_display_mode *mode)
+{
+       return _intel_panel_fixed_mode(connector, mode, NULL);
+}
+
 static bool is_alt_drrs_mode(const struct drm_display_mode *mode,
                             const struct drm_display_mode *preferred_mode)
 {
@@ -202,11 +230,28 @@ int intel_panel_compute_config(struct intel_atomic_state 
*state,
                               struct intel_connector *connector)
 {
        struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
-       const struct drm_display_mode *fixed_mode =
-               intel_panel_fixed_mode(connector, adjusted_mode);
+       const struct drm_display_mode *fixed_mode = NULL;
        int vrefresh, fixed_mode_vrefresh;
        bool is_vrr;
 
+       /*
+        * Attempt a VRR based refresh rate change if possible
+        * when userspace has forbidden a full modeset.
+        */
+       if (!state->base.allow_modeset) {
+               struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+               const struct intel_crtc_state *old_crtc_state =
+                       intel_atomic_get_old_crtc_state(state, crtc);
+
+               if (old_crtc_state->hw.enable &&
+                   old_crtc_state->uapi.encoder_mask == 
crtc_state->uapi.encoder_mask)
+                       fixed_mode = _intel_panel_fixed_mode(connector, 
adjusted_mode,
+                                                            
&old_crtc_state->hw.adjusted_mode);
+       }
+
+       if (!fixed_mode)
+               fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode);
+
        if (!fixed_mode)
                return 0;
 
-- 
2.53.0

Reply via email to