From eb4796d8625902adfd0bc7226306afcde617f7c9 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Thu, 27 Feb 2025 16:14:25 -0300 Subject: [PATCH 01/16] drm/i915/xe3lpd: Map POWER_DOMAIN_AUDIO_PLAYBACK to DC_off In Xe3_LPD, display audio has the core audio logic located in PG0 and per-transcoder logic in the same power well that provides power for the transcoder [1]. For stuff like audio device enumeration, we need to ensure that PG0 is turned on. For playback, we additionally need the transcoder's power well to be enabled. That essentially means that, for audio playback, there isn't a special power well that needs to be enabled, because modeset sequences will ensure that the required power wells are enabled. That said, there might be cases where PG0 could be disabled due to display entering DC6 while the audio driver tries to interact with the graphics driver for stuff like audio device enumeration. We recently hit that kind of scenario, where "aplay -l" was being used to enumerate audio devices on a PTL machine with PSR enabled and no external displays attached. Since intel_audio_component_get_power() uses POWER_DOMAIN_AUDIO_PLAYBACK, make sure to map that power domain to DC_off power well, so that we disable dynamic DC states (which includes DC6) while the audio driver needs display audio power. [1] The core-audio vs per-transcoder logic split is not really new in Xe3_LPD. This is also true for previous display generations. We need to figure out the correct version where this split happened so that we can apply fixes in the current power domain mapping. Bspec: 72519 Reviewed-by: Kai Vehmanen Link: https://patchwork.freedesktop.org/patch/msgid/20250227-xe3lpd-power-domain-audio-playback-v1-1-5765f21da977@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/i915/display/intel_display_power_map.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c index e80e1fd611ca..ab1163744bc5 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c @@ -1696,6 +1696,7 @@ I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_dc_off, XE3LPD_PW_C_POWER_DOMAINS, XE3LPD_PW_D_POWER_DOMAINS, POWER_DOMAIN_AUDIO_MMIO, + POWER_DOMAIN_AUDIO_PLAYBACK, POWER_DOMAIN_INIT); static const struct i915_power_well_desc xe3lpd_power_wells_dcoff[] = { -- 2.51.0 From 9f1e253d789649745db33a205969169033f078c9 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 4 Mar 2025 17:29:12 +0200 Subject: [PATCH 02/16] drm/i915/hpd: Track HPD pins instead of ports for HPD pulse events MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Track the HPD pin instead of the corresponding encoder ports for pending short/long HPD pulse events. This is how the pending hotplug events are tracked and there is no reason for tracking the pulse events differently. After this change intel_hpd_trigger_irq() will set the short pulse event pending for all encoders using the given HPD pin. This doesn't change the behavior, as atm in case of multiple (2) encoders sharing the same pin only one will have a pulse handler, so for other encoders without a pulse handler the event is ignored. Also setting the pulse event pending for all encoders using the HPD pin is what happens after an actual HPD IRQ, the effect of calling intel_hpd_trigger_irq() should match this. In a following change this also makes it simpler to block the handling of a short/long pulse event on an HPD pin for all the encoders using this HPD pin. Suggested-by: Ville Syrjälä Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250304152917.3407080-2-imre.deak@intel.com --- .../gpu/drm/i915/display/intel_display_core.h | 4 +-- drivers/gpu/drm/i915/display/intel_hotplug.c | 30 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index eeb7ae3eaea8..afb2184bf233 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -170,8 +170,8 @@ struct intel_hotplug { u32 retry_bits; struct delayed_work reenable_work; - u32 long_port_mask; - u32 short_port_mask; + u32 long_hpd_pin_mask; + u32 short_hpd_pin_mask; struct work_struct dig_port_work; struct work_struct poll_init_work; diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 00d7b1ccf190..9692b5c01aea 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -353,28 +353,28 @@ static void i915_digport_work_func(struct work_struct *work) { struct drm_i915_private *dev_priv = container_of(work, struct drm_i915_private, display.hotplug.dig_port_work); - u32 long_port_mask, short_port_mask; + u32 long_hpd_pin_mask, short_hpd_pin_mask; struct intel_encoder *encoder; u32 old_bits = 0; spin_lock_irq(&dev_priv->irq_lock); - long_port_mask = dev_priv->display.hotplug.long_port_mask; - dev_priv->display.hotplug.long_port_mask = 0; - short_port_mask = dev_priv->display.hotplug.short_port_mask; - dev_priv->display.hotplug.short_port_mask = 0; + long_hpd_pin_mask = dev_priv->display.hotplug.long_hpd_pin_mask; + dev_priv->display.hotplug.long_hpd_pin_mask = 0; + short_hpd_pin_mask = dev_priv->display.hotplug.short_hpd_pin_mask; + dev_priv->display.hotplug.short_hpd_pin_mask = 0; spin_unlock_irq(&dev_priv->irq_lock); for_each_intel_encoder(&dev_priv->drm, encoder) { struct intel_digital_port *dig_port; - enum port port = encoder->port; + enum hpd_pin pin = encoder->hpd_pin; bool long_hpd, short_hpd; enum irqreturn ret; if (!intel_encoder_has_hpd_pulse(encoder)) continue; - long_hpd = long_port_mask & BIT(port); - short_hpd = short_port_mask & BIT(port); + long_hpd = long_hpd_pin_mask & BIT(pin); + short_hpd = short_hpd_pin_mask & BIT(pin); if (!long_hpd && !short_hpd) continue; @@ -384,7 +384,7 @@ static void i915_digport_work_func(struct work_struct *work) ret = dig_port->hpd_pulse(dig_port, long_hpd); if (ret == IRQ_NONE) { /* fall back to old school hpd */ - old_bits |= BIT(encoder->hpd_pin); + old_bits |= BIT(pin); } } @@ -407,9 +407,10 @@ static void i915_digport_work_func(struct work_struct *work) void intel_hpd_trigger_irq(struct intel_digital_port *dig_port) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + struct intel_encoder *encoder = &dig_port->base; spin_lock_irq(&i915->irq_lock); - i915->display.hotplug.short_port_mask |= BIT(dig_port->base.port); + i915->display.hotplug.short_hpd_pin_mask |= BIT(encoder->hpd_pin); spin_unlock_irq(&i915->irq_lock); queue_work(i915->display.hotplug.dp_wq, &i915->display.hotplug.dig_port_work); @@ -557,7 +558,6 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, * only the one of them (DP) will have ->hpd_pulse(). */ for_each_intel_encoder(&dev_priv->drm, encoder) { - enum port port = encoder->port; bool long_hpd; pin = encoder->hpd_pin; @@ -577,10 +577,10 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, if (long_hpd) { long_hpd_pulse_mask |= BIT(pin); - dev_priv->display.hotplug.long_port_mask |= BIT(port); + dev_priv->display.hotplug.long_hpd_pin_mask |= BIT(pin); } else { short_hpd_pulse_mask |= BIT(pin); - dev_priv->display.hotplug.short_port_mask |= BIT(port); + dev_priv->display.hotplug.short_hpd_pin_mask |= BIT(pin); } } @@ -920,8 +920,8 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) spin_lock_irq(&dev_priv->irq_lock); - dev_priv->display.hotplug.long_port_mask = 0; - dev_priv->display.hotplug.short_port_mask = 0; + dev_priv->display.hotplug.long_hpd_pin_mask = 0; + dev_priv->display.hotplug.short_hpd_pin_mask = 0; dev_priv->display.hotplug.event_bits = 0; dev_priv->display.hotplug.retry_bits = 0; -- 2.51.0 From 4b16619608ff14338b6001acb810506079c49749 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 4 Mar 2025 17:29:13 +0200 Subject: [PATCH 03/16] drm/i915/hpd: Let an HPD pin be in the disabled state when handling missed IRQs After suspending and resuming the detection on connectors, HPD IRQs that arrived while the detection was suspended, are handled by scheduling the intel_hotplug::hotplug work for them. All HPD pins must be at this point in either the HPD_ENABLED (set for all pins during driver loading/system resuming) or HPD_MARK_DISABLED (set by IRQ storm detection) state: the HPD_DISABLED state for a pin can be set only from the HPD_MARK_DISABLED state by the hotplug work after a storm detection (enabling polling on the given pin/connector), however the hotplug work won't be scheduled while the detection is suspended. A follow-up change will add support for blocking the HPD IRQ handling on a given HPD pin (without disabling the IRQ generation on it), after which it becomes possible to see a pin in the HPD_DISABLED state when unblocking the IRQ handling (since the blocking could've happened for an already disabled pin). Adjust queue_work_for_missed_irqs() accordingly, so that this function can be reused for unblocking the IRQ handling. Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250304152917.3407080-3-imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 9692b5c01aea..3fb5feeefa14 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -980,6 +980,7 @@ static void queue_work_for_missed_irqs(struct drm_i915_private *i915) case HPD_MARK_DISABLED: queue_work = true; break; + case HPD_DISABLED: case HPD_ENABLED: break; default: -- 2.51.0 From 0d77a3e0ea90a7ee25755a94694cdfd822c9db6b Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Wed, 5 Mar 2025 13:48:19 +0200 Subject: [PATCH 04/16] drm/i915/hpd: Add support for blocking the IRQ handling on an HPD pin MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add support for blocking the IRQ handling on the HPD pin of a given encoder, handling IRQs that arrived while in the blocked state after unblocking the IRQ handling. This will be used by a follow-up change, which blocks/unblocks the IRQ handling around DP link training. This is similar to the intel_hpd_disable/enable() functionality, by also handling encoders/ports with a pulse handler (i.e. also blocking/unblocking the short/long pulse handling) and handling the IRQs arrived in the blocked state after the handling is unblocked (vs. just dropping such IRQs). v2: - Handle encoders without a port assigned to them. - Fix clearing IRQs from intel_hotplug::short_port_mask. v3: - Rename intel_hpd_suspend/resume() to intel_hpd_block/unblock(). (Jani) - Refer to HPD pins as hpd_pin vs. hpd. - Flush dig_port_work in intel_hpd_block() if any encoder using the HPD pin has a pulse handler. v4: - Fix hpd_pin_has_pulse(), checking the encoder's HPD pin. v5: - Rebase on port->hpd_pin tracking. (Ville) v6: (Jani) - Add hpd_pin_is_blocked() helper. - Use the hpd_pin_mask term for a mask of pins instead of hpd_pins. - Prevent decrementing a 0 refcount in unblock_hpd_pin(). Cc: Jani Nikula Cc: Ville Syrjälä Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250305114820.3523077-1-imre.deak@intel.com --- .../gpu/drm/i915/display/intel_display_core.h | 1 + drivers/gpu/drm/i915/display/intel_hotplug.c | 210 +++++++++++++++--- drivers/gpu/drm/i915/display/intel_hotplug.h | 2 + 3 files changed, 188 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h index afb2184bf233..3673275f9061 100644 --- a/drivers/gpu/drm/i915/display/intel_display_core.h +++ b/drivers/gpu/drm/i915/display/intel_display_core.h @@ -160,6 +160,7 @@ struct intel_hotplug { struct { unsigned long last_jiffies; int count; + int blocked_count; enum { HPD_ENABLED = 0, HPD_DISABLED = 1, diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 3fb5feeefa14..94b4dcf10f58 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -349,19 +349,62 @@ static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder) enc_to_dig_port(encoder)->hpd_pulse != NULL; } +static bool hpd_pin_has_pulse(struct intel_display *display, enum hpd_pin pin) +{ + struct intel_encoder *encoder; + + for_each_intel_encoder(display->drm, encoder) { + if (encoder->hpd_pin != pin) + continue; + + if (intel_encoder_has_hpd_pulse(encoder)) + return true; + } + + return false; +} + +static bool hpd_pin_is_blocked(struct intel_display *display, enum hpd_pin pin) +{ + struct drm_i915_private *i915 = to_i915(display->drm); + + lockdep_assert_held(&i915->irq_lock); + + return display->hotplug.stats[pin].blocked_count; +} + +static u32 get_blocked_hpd_pin_mask(struct intel_display *display) +{ + enum hpd_pin pin; + u32 hpd_pin_mask = 0; + + for_each_hpd_pin(pin) { + if (hpd_pin_is_blocked(display, pin)) + hpd_pin_mask |= BIT(pin); + } + + return hpd_pin_mask; +} + static void i915_digport_work_func(struct work_struct *work) { - struct drm_i915_private *dev_priv = - container_of(work, struct drm_i915_private, display.hotplug.dig_port_work); + struct intel_display *display = + container_of(work, struct intel_display, hotplug.dig_port_work); + struct drm_i915_private *dev_priv = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; u32 long_hpd_pin_mask, short_hpd_pin_mask; struct intel_encoder *encoder; + u32 blocked_hpd_pin_mask; u32 old_bits = 0; spin_lock_irq(&dev_priv->irq_lock); - long_hpd_pin_mask = dev_priv->display.hotplug.long_hpd_pin_mask; - dev_priv->display.hotplug.long_hpd_pin_mask = 0; - short_hpd_pin_mask = dev_priv->display.hotplug.short_hpd_pin_mask; - dev_priv->display.hotplug.short_hpd_pin_mask = 0; + + blocked_hpd_pin_mask = get_blocked_hpd_pin_mask(display); + long_hpd_pin_mask = hotplug->long_hpd_pin_mask & ~blocked_hpd_pin_mask; + hotplug->long_hpd_pin_mask &= ~long_hpd_pin_mask; + short_hpd_pin_mask = hotplug->short_hpd_pin_mask & ~blocked_hpd_pin_mask; + hotplug->short_hpd_pin_mask &= ~short_hpd_pin_mask; + spin_unlock_irq(&dev_priv->irq_lock); for_each_intel_encoder(&dev_priv->drm, encoder) { @@ -406,14 +449,18 @@ static void i915_digport_work_func(struct work_struct *work) */ void intel_hpd_trigger_irq(struct intel_digital_port *dig_port) { - struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + struct intel_display *display = to_intel_display(dig_port); + struct drm_i915_private *i915 = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; struct intel_encoder *encoder = &dig_port->base; spin_lock_irq(&i915->irq_lock); - i915->display.hotplug.short_hpd_pin_mask |= BIT(encoder->hpd_pin); - spin_unlock_irq(&i915->irq_lock); - queue_work(i915->display.hotplug.dp_wq, &i915->display.hotplug.dig_port_work); + hotplug->short_hpd_pin_mask |= BIT(encoder->hpd_pin); + if (!hpd_pin_is_blocked(display, encoder->hpd_pin)) + queue_work(hotplug->dp_wq, &hotplug->dig_port_work); + + spin_unlock_irq(&i915->irq_lock); } /* @@ -421,9 +468,10 @@ void intel_hpd_trigger_irq(struct intel_digital_port *dig_port) */ static void i915_hotplug_work_func(struct work_struct *work) { - struct drm_i915_private *dev_priv = - container_of(work, struct drm_i915_private, - display.hotplug.hotplug_work.work); + struct intel_display *display = + container_of(work, struct intel_display, hotplug.hotplug_work.work); + struct drm_i915_private *dev_priv = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; struct drm_connector_list_iter conn_iter; struct intel_connector *connector; u32 changed = 0, retry = 0; @@ -431,16 +479,18 @@ static void i915_hotplug_work_func(struct work_struct *work) u32 hpd_retry_bits; struct drm_connector *first_changed_connector = NULL; int changed_connectors = 0; + u32 blocked_hpd_pin_mask; mutex_lock(&dev_priv->drm.mode_config.mutex); drm_dbg_kms(&dev_priv->drm, "running encoder hotplug functions\n"); spin_lock_irq(&dev_priv->irq_lock); - hpd_event_bits = dev_priv->display.hotplug.event_bits; - dev_priv->display.hotplug.event_bits = 0; - hpd_retry_bits = dev_priv->display.hotplug.retry_bits; - dev_priv->display.hotplug.retry_bits = 0; + blocked_hpd_pin_mask = get_blocked_hpd_pin_mask(display); + hpd_event_bits = hotplug->event_bits & ~blocked_hpd_pin_mask; + hotplug->event_bits &= ~hpd_event_bits; + hpd_retry_bits = hotplug->retry_bits & ~blocked_hpd_pin_mask; + hotplug->retry_bits &= ~hpd_retry_bits; /* Enable polling for connectors which had HPD IRQ storms */ intel_hpd_irq_storm_switch_to_polling(dev_priv); @@ -539,6 +589,7 @@ static void i915_hotplug_work_func(struct work_struct *work) void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, u32 pin_mask, u32 long_mask) { + struct intel_display *display = to_intel_display(&dev_priv->drm); struct intel_encoder *encoder; bool storm_detected = false; bool queue_dig = false, queue_hp = false; @@ -573,7 +624,9 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, "digital hpd on [ENCODER:%d:%s] - %s\n", encoder->base.base.id, encoder->base.name, long_hpd ? "long" : "short"); - queue_dig = true; + + if (!hpd_pin_is_blocked(display, pin)) + queue_dig = true; if (long_hpd) { long_hpd_pulse_mask |= BIT(pin); @@ -617,7 +670,9 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv, } else { dev_priv->display.hotplug.event_bits |= BIT(pin); long_hpd = true; - queue_hp = true; + + if (!hpd_pin_is_blocked(display, pin)) + queue_hp = true; } if (intel_hpd_irq_storm_detect(dev_priv, pin, long_hpd)) { @@ -915,11 +970,15 @@ static bool cancel_all_detection_work(struct drm_i915_private *i915) void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) { + struct intel_display *display = to_intel_display(&dev_priv->drm); + if (!HAS_DISPLAY(dev_priv)) return; spin_lock_irq(&dev_priv->irq_lock); + drm_WARN_ON(display->drm, get_blocked_hpd_pin_mask(display)); + dev_priv->display.hotplug.long_hpd_pin_mask = 0; dev_priv->display.hotplug.short_hpd_pin_mask = 0; dev_priv->display.hotplug.event_bits = 0; @@ -966,19 +1025,22 @@ void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin) static void queue_work_for_missed_irqs(struct drm_i915_private *i915) { - bool queue_work = false; + struct intel_display *display = to_intel_display(&i915->drm); + struct intel_hotplug *hotplug = &display->hotplug; + bool queue_hp_work = false; + u32 blocked_hpd_pin_mask; enum hpd_pin pin; lockdep_assert_held(&i915->irq_lock); - if (i915->display.hotplug.event_bits || - i915->display.hotplug.retry_bits) - queue_work = true; + blocked_hpd_pin_mask = get_blocked_hpd_pin_mask(display); + if ((hotplug->event_bits | hotplug->retry_bits) & ~blocked_hpd_pin_mask) + queue_hp_work = true; for_each_hpd_pin(pin) { switch (i915->display.hotplug.stats[pin].state) { case HPD_MARK_DISABLED: - queue_work = true; + queue_hp_work = true; break; case HPD_DISABLED: case HPD_ENABLED: @@ -988,10 +1050,108 @@ static void queue_work_for_missed_irqs(struct drm_i915_private *i915) } } - if (queue_work) + if ((hotplug->long_hpd_pin_mask | hotplug->short_hpd_pin_mask) & ~blocked_hpd_pin_mask) + queue_work(hotplug->dp_wq, &hotplug->dig_port_work); + + if (queue_hp_work) queue_delayed_detection_work(i915, &i915->display.hotplug.hotplug_work, 0); } +static bool block_hpd_pin(struct intel_display *display, enum hpd_pin pin) +{ + struct drm_i915_private *i915 = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; + + lockdep_assert_held(&i915->irq_lock); + + hotplug->stats[pin].blocked_count++; + + return hotplug->stats[pin].blocked_count == 1; +} + +static bool unblock_hpd_pin(struct intel_display *display, enum hpd_pin pin) +{ + struct drm_i915_private *i915 = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; + + lockdep_assert_held(&i915->irq_lock); + + if (drm_WARN_ON(display->drm, hotplug->stats[pin].blocked_count == 0)) + return true; + + hotplug->stats[pin].blocked_count--; + + return hotplug->stats[pin].blocked_count == 0; +} + +/** + * intel_hpd_block - Block handling of HPD IRQs on an HPD pin + * @encoder: Encoder to block the HPD handling for + * + * Blocks the handling of HPD IRQs on the HPD pin of @encoder. + * + * On return: + * - It's guaranteed that the blocked encoders' HPD pulse handler + * (via intel_digital_port::hpd_pulse()) is not running. + * - The hotplug event handling (via intel_encoder::hotplug()) of an + * HPD IRQ pending at the time this function is called may be still + * running. + * - Detection on the encoder's connector (via + * drm_connector_helper_funcs::detect_ctx(), + * drm_connector_funcs::detect()) remains allowed, for instance as part of + * userspace connector probing, or DRM core's connector polling. + * + * The call must be followed by calling intel_hpd_unblock(). + * + * Note that the handling of HPD IRQs for another encoder using the same HPD + * pin as that of @encoder will be also blocked. + */ +void intel_hpd_block(struct intel_encoder *encoder) +{ + struct intel_display *display = to_intel_display(encoder); + struct drm_i915_private *i915 = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; + bool do_flush = false; + + if (encoder->hpd_pin == HPD_NONE) + return; + + spin_lock_irq(&i915->irq_lock); + + if (block_hpd_pin(display, encoder->hpd_pin)) + do_flush = true; + + spin_unlock_irq(&i915->irq_lock); + + if (do_flush && hpd_pin_has_pulse(display, encoder->hpd_pin)) + flush_work(&hotplug->dig_port_work); +} + +/** + * intel_hpd_unblock - Unblock handling of HPD IRQs on an HPD pin + * @encoder: Encoder to unblock the HPD handling for + * + * Unblock the handling of HPD IRQs on the HPD pin of @encoder, which was + * previously blocked by intel_hpd_block(). Any HPD IRQ raised on the + * HPD pin while it was blocked will be handled for @encoder and for any + * other encoder sharing the same HPD pin. + */ +void intel_hpd_unblock(struct intel_encoder *encoder) +{ + struct intel_display *display = to_intel_display(encoder); + struct drm_i915_private *i915 = to_i915(display->drm); + + if (encoder->hpd_pin == HPD_NONE) + return; + + spin_lock_irq(&i915->irq_lock); + + if (unblock_hpd_pin(display, encoder->hpd_pin)) + queue_work_for_missed_irqs(i915); + + spin_unlock_irq(&i915->irq_lock); +} + void intel_hpd_enable_detection_work(struct drm_i915_private *i915) { spin_lock_irq(&i915->irq_lock); diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h index d6986902b054..5f9857136f5e 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.h +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h @@ -28,6 +28,8 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); enum hpd_pin intel_hpd_pin_default(enum port port); bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin); void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin); +void intel_hpd_block(struct intel_encoder *encoder); +void intel_hpd_unblock(struct intel_encoder *encoder); void intel_hpd_debugfs_register(struct drm_i915_private *i915); void intel_hpd_enable_detection_work(struct drm_i915_private *i915); -- 2.51.0 From 35021b5b15de0c4eceecda9e2dadab2e5e56b7e2 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 4 Mar 2025 17:29:15 +0200 Subject: [PATCH 05/16] drm/i915/dp: Fix link training interrupted by a short HPD pulse During Display Port link training the handling of HPD pulses should be prevented, as that handling can interfere with the link training: - Accessing DPCD registers outside the range of link training registers are not allowed by the Standard (see DP Standard v2.1, 3.5.2.16.1, 3.6.6.1). The pulse handler reads the DPRX capability registers, which are outside of the allowed range. - Switching of the LTTPR transparent/non-transparent mode may reset the LTTPRs on the link, thus aborting any ongoing link training. The pulse handler does set the LTTPR mode, thus it could unexpectedly abort the ongoing link training. Block/unblock the HPD pulse handling for the duration of the link training to prevent the above DPCD register accesses / LTTPR mode change. Apart from the above scenarios, there are other ways a non-link training DPCD register could be accessed during link training: via the DRM AUX device node, or via DPCD register probing (as performed by drm_dp_dpcd_probe()). These will be addressed by a follow-up change. v2: Rebase on the intel_hpd_suspend/resume -> intel_hpd_block/unblock() rename change. Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250304152917.3407080-5-imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_dp_link_training.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 581f1dab618e..5d549ac4de1c 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -1124,6 +1124,8 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp, void intel_dp_stop_link_train(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; + intel_dp->link_trained = true; intel_dp_disable_dpcd_training_pattern(intel_dp, DP_PHY_DPRX); @@ -1134,6 +1136,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp, wait_for(intel_dp_128b132b_intra_hop(intel_dp, crtc_state) == 0, 500)) { lt_dbg(intel_dp, DP_PHY_DPRX, "128b/132b intra-hop not clearing\n"); } + + intel_hpd_unblock(encoder); } static bool @@ -1616,7 +1620,11 @@ void intel_dp_start_link_train(struct intel_atomic_state *state, * non-transparent mode. During an earlier LTTPR detection this * could've been prevented by an active link. */ - int lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp); + int lttpr_count; + + intel_hpd_block(encoder); + + lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp); if (lttpr_count < 0) /* Still continue with enabling the port and link training. */ -- 2.51.0 From 29c09cf200f736138707857696d1f6db2db0299b Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Wed, 5 Mar 2025 13:48:20 +0200 Subject: [PATCH 06/16] drm/i915/dp: Queue a link check after link training is complete After link training - both in case of a passing and failing LT result - a work is scheduled to check the link state. This check should take place after the link training is completed by disabling the link training pattern and setting intel_dp::link_trained=true. Atm, the work is scheduled before these steps, which may result in checking the link state too early (and thus not retraining the link as expected). Fix the above by scheduling the link check work after link training is complete. v2: - Add MAX_SEQ_TRAIN_FAILURES instead of open-coding it. (Jani) Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250305114820.3523077-2-imre.deak@intel.com --- .../gpu/drm/i915/display/intel_dp_link_training.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 5d549ac4de1c..ded246bbf232 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -56,6 +56,8 @@ lt_dbg(_intel_dp, _dp_phy, "Sink disconnected: " _format, ## __VA_ARGS__); \ } while (0) +#define MAX_SEQ_TRAIN_FAILURES 2 + static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp) { memset(intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps)); @@ -1124,6 +1126,7 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp, void intel_dp_stop_link_train(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { + struct intel_display *display = to_intel_display(intel_dp); struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; intel_dp->link_trained = true; @@ -1138,6 +1141,13 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp, } intel_hpd_unblock(encoder); + + if (!display->hotplug.ignore_long_hpd && + intel_dp->link.seq_train_failures < MAX_SEQ_TRAIN_FAILURES) { + int delay_ms = intel_dp->link.seq_train_failures ? 0 : 2000; + + intel_encoder_link_check_queue_work(encoder, delay_ms); + } } static bool @@ -1642,7 +1652,6 @@ void intel_dp_start_link_train(struct intel_atomic_state *state, lt_dbg(intel_dp, DP_PHY_DPRX, "Forcing link training failure\n"); } else if (passed) { intel_dp->link.seq_train_failures = 0; - intel_encoder_link_check_queue_work(encoder, 2000); return; } @@ -1665,10 +1674,8 @@ void intel_dp_start_link_train(struct intel_atomic_state *state, return; } - if (intel_dp->link.seq_train_failures < 2) { - intel_encoder_link_check_queue_work(encoder, 0); + if (intel_dp->link.seq_train_failures < MAX_SEQ_TRAIN_FAILURES) return; - } if (intel_dp_schedule_fallback_link_training(state, intel_dp, crtc_state)) return; -- 2.51.0 From 6ace085c453ccdcad34e64eead21eb120270c383 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 4 Mar 2025 17:29:17 +0200 Subject: [PATCH 07/16] drm/i915/crt: Use intel_hpd_block/unblock() instead of intel_hpd_disable/enable() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit intel_hpd_disable/enable() have the same purpose as intel_hpd_block/unblock(), except that disable/enable will drop any HPD IRQs which were triggered while the HPD was disabled, while block/unblock will handle such IRQs after the IRQ handling is unblocked. Use intel_hpd_block/unblock() for crt as well, by adding a helper to explicitly clear any pending IRQs before unblocking. v2: - Handle encoders without a port assigned to them. - Rebase on change in intel_hpd_suspend() documentation. v3: - Rebase on the suspend/resume -> block/unblock rename change. - Clear the pending events only after all encoders have unblocked the HPD handling. - Clear the short/long port events for all encoders using the given HPD pin. v4: - Rebase on port->hpd_pin tracking. (Ville) Cc: Ville Syrjälä Reviewed-by: Jani Nikula Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20250304152917.3407080-7-imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_crt.c | 7 +-- drivers/gpu/drm/i915/display/intel_hotplug.c | 60 +++++++++++--------- drivers/gpu/drm/i915/display/intel_hotplug.h | 3 +- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c index 76ffb3f8467c..bca91d49cb96 100644 --- a/drivers/gpu/drm/i915/display/intel_crt.c +++ b/drivers/gpu/drm/i915/display/intel_crt.c @@ -532,8 +532,6 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) { struct intel_display *display = to_intel_display(connector->dev); struct intel_crt *crt = intel_attached_crt(to_intel_connector(connector)); - struct drm_i915_private *dev_priv = to_i915(connector->dev); - bool reenable_hpd; u32 adpa; bool ret; u32 save_adpa; @@ -550,7 +548,7 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) * * Just disable HPD interrupts here to prevent this */ - reenable_hpd = intel_hpd_disable(dev_priv, crt->base.hpd_pin); + intel_hpd_block(&crt->base); save_adpa = adpa = intel_de_read(display, crt->adpa_reg); drm_dbg_kms(display->drm, @@ -577,8 +575,7 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) drm_dbg_kms(display->drm, "valleyview hotplug adpa=0x%x, result %d\n", adpa, ret); - if (reenable_hpd) - intel_hpd_enable(dev_priv, crt->base.hpd_pin); + intel_hpd_clear_and_unblock(&crt->base); return ret; } diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 94b4dcf10f58..c69b1f5fd160 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -996,33 +996,6 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) drm_dbg_kms(&dev_priv->drm, "Hotplug detection work still active\n"); } -bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin) -{ - bool ret = false; - - if (pin == HPD_NONE) - return false; - - spin_lock_irq(&dev_priv->irq_lock); - if (dev_priv->display.hotplug.stats[pin].state == HPD_ENABLED) { - dev_priv->display.hotplug.stats[pin].state = HPD_DISABLED; - ret = true; - } - spin_unlock_irq(&dev_priv->irq_lock); - - return ret; -} - -void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin) -{ - if (pin == HPD_NONE) - return; - - spin_lock_irq(&dev_priv->irq_lock); - dev_priv->display.hotplug.stats[pin].state = HPD_ENABLED; - spin_unlock_irq(&dev_priv->irq_lock); -} - static void queue_work_for_missed_irqs(struct drm_i915_private *i915) { struct intel_display *display = to_intel_display(&i915->drm); @@ -1101,7 +1074,8 @@ static bool unblock_hpd_pin(struct intel_display *display, enum hpd_pin pin) * drm_connector_funcs::detect()) remains allowed, for instance as part of * userspace connector probing, or DRM core's connector polling. * - * The call must be followed by calling intel_hpd_unblock(). + * The call must be followed by calling intel_hpd_unblock(), or + * intel_hpd_clear_and_unblock(). * * Note that the handling of HPD IRQs for another encoder using the same HPD * pin as that of @encoder will be also blocked. @@ -1152,6 +1126,36 @@ void intel_hpd_unblock(struct intel_encoder *encoder) spin_unlock_irq(&i915->irq_lock); } +/** + * intel_hpd_clear_and_unblock - Unblock handling of new HPD IRQs on an HPD pin + * @encoder: Encoder to unblock the HPD handling for + * + * Unblock the handling of HPD IRQs on the HPD pin of @encoder, which was + * previously blocked by intel_hpd_block(). Any HPD IRQ raised on the + * HPD pin while it was blocked will be cleared, handling only new IRQs. + */ +void intel_hpd_clear_and_unblock(struct intel_encoder *encoder) +{ + struct intel_display *display = to_intel_display(encoder); + struct drm_i915_private *i915 = to_i915(display->drm); + struct intel_hotplug *hotplug = &display->hotplug; + enum hpd_pin pin = encoder->hpd_pin; + + if (pin == HPD_NONE) + return; + + spin_lock_irq(&i915->irq_lock); + + if (unblock_hpd_pin(display, pin)) { + hotplug->event_bits &= ~BIT(pin); + hotplug->retry_bits &= ~BIT(pin); + hotplug->short_hpd_pin_mask &= ~BIT(pin); + hotplug->long_hpd_pin_mask &= ~BIT(pin); + } + + spin_unlock_irq(&i915->irq_lock); +} + void intel_hpd_enable_detection_work(struct drm_i915_private *i915) { spin_lock_irq(&i915->irq_lock); diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h index 5f9857136f5e..f189b871904e 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.h +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h @@ -26,10 +26,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv); void intel_hpd_init_early(struct drm_i915_private *i915); void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); enum hpd_pin intel_hpd_pin_default(enum port port); -bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin); -void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin); void intel_hpd_block(struct intel_encoder *encoder); void intel_hpd_unblock(struct intel_encoder *encoder); +void intel_hpd_clear_and_unblock(struct intel_encoder *encoder); void intel_hpd_debugfs_register(struct drm_i915_private *i915); void intel_hpd_enable_detection_work(struct drm_i915_private *i915); -- 2.51.0 From 5d6c69b712f9cb34063ef32168ce6a12af8acf0c Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Thu, 27 Feb 2025 09:11:06 +0530 Subject: [PATCH 08/16] drm/i915/watermark: Check bounds for scaler_users for dsc prefill latency Currently, during the computation of global watermarks, the latency for each scaler user is calculated to compute the DSC prefill latency. At this point, the number of scaler users can exceed the number of supported scalers, which is checked later in intel_atomic_setup_scalers(). This can cause issues when the number of scaler users exceeds the number of supported scalers. While checking for DSC prefill, ensure that the number of scaler users does not exceed the number of supported scalers. Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4341 Fixes: a9b14af999b0 ("drm/i915/dsc: Check if vblank is sufficient for dsc prefill") Cc: Mitul Golani Cc: Ankit Nautiyal Cc: Jani Nikula Signed-off-by: Ankit Nautiyal Reviewed-by: Mitul Golani Link: https://patchwork.freedesktop.org/patch/msgid/20250227034106.1638203-1-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/skl_watermark.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index 2d0de1c63308..621e97943542 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2314,6 +2314,7 @@ cdclk_prefill_adjustment(const struct intel_crtc_state *crtc_state) static int dsc_prefill_latency(const struct intel_crtc_state *crtc_state) { + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); const struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state; int linetime = DIV_ROUND_UP(1000 * crtc_state->hw.adjusted_mode.htotal, @@ -2323,7 +2324,9 @@ dsc_prefill_latency(const struct intel_crtc_state *crtc_state) crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ? 2 : 1; u32 dsc_prefill_latency = 0; - if (!crtc_state->dsc.compression_enable || !num_scaler_users) + if (!crtc_state->dsc.compression_enable || + !num_scaler_users || + num_scaler_users > crtc->num_scalers) return dsc_prefill_latency; dsc_prefill_latency = DIV_ROUND_UP(15 * linetime * chroma_downscaling_factor, 10); -- 2.51.0 From 03710f3d063d8f4873ef43d030bea375243bcbe4 Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:44 +0530 Subject: [PATCH 09/16] drm/i915/vrr: Remove unwanted comment MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The comment about fixed average vtotal is incorrect. Remove it. Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-2-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index cac49319026d..106bfaf6649b 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -276,11 +276,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, */ crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display); - /* - * When panel is VRR capable and userspace has - * not enabled adaptive sync mode then Fixed Average - * Vtotal mode should be enabled. - */ if (crtc_state->uapi.vrr_enabled) { crtc_state->vrr.enable = true; crtc_state->mode_flags |= I915_MODE_FLAG_VRR; -- 2.51.0 From 022d04b355a2771bc3de970a7f14980a716bfe4c Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:45 +0530 Subject: [PATCH 10/16] drm/i915:vrr: Separate out functions to compute vmin and vmax MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Make helpers to compute vmin and vmax. v2: Make the adjusted mode const (Ville) Use reverse xmas tree order of declarations. (Ville) Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-3-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 38 +++++++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 106bfaf6649b..a88b77114867 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -222,6 +222,34 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required) return vtotal; } +static +int intel_vrr_compute_vmin(struct intel_connector *connector, + const struct drm_display_mode *adjusted_mode) +{ + const struct drm_display_info *info = &connector->base.display_info; + int vmin; + + vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, + adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq); + vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal); + + return vmin; +} + +static +int intel_vrr_compute_vmax(struct intel_connector *connector, + const struct drm_display_mode *adjusted_mode) +{ + const struct drm_display_info *info = &connector->base.display_info; + int vmax; + + vmax = adjusted_mode->crtc_clock * 1000 / + (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq); + vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal); + + return vmax; +} + void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state) @@ -232,7 +260,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, struct intel_dp *intel_dp = intel_attached_dp(connector); bool is_edp = intel_dp_is_edp(intel_dp); struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; - const struct drm_display_info *info = &connector->base.display_info; int vmin, vmax; /* @@ -253,13 +280,8 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, if (HAS_LRR(display)) crtc_state->update_lrr = true; - vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, - adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq); - vmax = adjusted_mode->crtc_clock * 1000 / - (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq); - - vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal); - vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal); + vmin = intel_vrr_compute_vmin(connector, adjusted_mode); + vmax = intel_vrr_compute_vmax(connector, adjusted_mode); if (vmin >= vmax) return; -- 2.51.0 From 58f9466c8292f8319158eb4b0f5fcbe89709d499 Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:46 +0530 Subject: [PATCH 11/16] drm/i915/vrr: Make helpers for cmrr and vrr timings MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Separate out functions for computing cmrr and vrr timings. Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-4-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 45 +++++++++++++++--------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index a88b77114867..db0ea206e26e 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -222,6 +222,30 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required) return vtotal; } +static +void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state) +{ + crtc_state->vrr.enable = true; + crtc_state->cmrr.enable = true; + /* + * TODO: Compute precise target refresh rate to determine + * if video_mode_required should be true. Currently set to + * false due to uncertainty about the precise target + * refresh Rate. + */ + crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state, false); + crtc_state->vrr.vmin = crtc_state->vrr.vmax; + crtc_state->vrr.flipline = crtc_state->vrr.vmin; + crtc_state->mode_flags |= I915_MODE_FLAG_VRR; +} + +static +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) +{ + crtc_state->vrr.enable = true; + crtc_state->mode_flags |= I915_MODE_FLAG_VRR; +} + static int intel_vrr_compute_vmin(struct intel_connector *connector, const struct drm_display_mode *adjusted_mode) @@ -298,23 +322,10 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, */ crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display); - if (crtc_state->uapi.vrr_enabled) { - crtc_state->vrr.enable = true; - crtc_state->mode_flags |= I915_MODE_FLAG_VRR; - } else if (is_cmrr_frac_required(crtc_state) && is_edp) { - crtc_state->vrr.enable = true; - crtc_state->cmrr.enable = true; - /* - * TODO: Compute precise target refresh rate to determine - * if video_mode_required should be true. Currently set to - * false due to uncertainty about the precise target - * refresh Rate. - */ - crtc_state->vrr.vmax = cmrr_get_vtotal(crtc_state, false); - crtc_state->vrr.vmin = crtc_state->vrr.vmax; - crtc_state->vrr.flipline = crtc_state->vrr.vmin; - crtc_state->mode_flags |= I915_MODE_FLAG_VRR; - } + if (crtc_state->uapi.vrr_enabled) + intel_vrr_compute_vrr_timings(crtc_state); + else if (is_cmrr_frac_required(crtc_state) && is_edp) + intel_vrr_compute_cmrr_timings(crtc_state); if (HAS_AS_SDP(display)) { crtc_state->vrr.vsync_start = -- 2.51.0 From a15b20e5094abd3bed90edfc22bf9ff84ef99e3c Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:47 +0530 Subject: [PATCH 12/16] drm/i915/vrr: Disable CMRR MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Switching between variable and fixed timings is possible as for that we just need to flip between VRR timings. However for CMRR along with the timings, few other bits also need to be changed on the fly, which might cause issues. So disable CMRR for now, till we have variable and fixed timings sorted out. Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-5-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index db0ea206e26e..a57659820f4b 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -182,7 +182,8 @@ is_cmrr_frac_required(struct intel_crtc_state *crtc_state) int calculated_refresh_k, actual_refresh_k, pixel_clock_per_line; struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; - if (!HAS_CMRR(display)) + /* Avoid CMRR for now till we have VRR with fixed timings working */ + if (!HAS_CMRR(display) || true) return false; actual_refresh_k = -- 2.51.0 From 27217f9d185666c8bc1449796cd4029ca66d8d3c Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:48 +0530 Subject: [PATCH 13/16] drm/i915/vrr: Track vrr.enable only for variable timing MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Since CMRR is now disabled, use the flag vrr.enable to tracks if vrr timing generator is used with variable timings. Avoid setting vrr.enable for CMRR and adjust readout to not set vrr.enable when vmax == vmin == flipline (fixed refresh rate timing). v2: Use intel_vrr_vmin_flipline() to account for adjustments required for icl/tgl. (Ville) v3: Add a #TODO for handling I915_MODE_FLAG_VRR better for CMRR. (Ville) Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-6-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index a57659820f4b..7320eb97991f 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -226,7 +226,6 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required) static void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state) { - crtc_state->vrr.enable = true; crtc_state->cmrr.enable = true; /* * TODO: Compute precise target refresh rate to determine @@ -527,6 +526,14 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); } +static +bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) +{ + return crtc_state->vrr.flipline && + crtc_state->vrr.flipline == crtc_state->vrr.vmax && + crtc_state->vrr.flipline == intel_vrr_vmin_flipline(crtc_state); +} + void intel_vrr_get_config(struct intel_crtc_state *crtc_state) { struct intel_display *display = to_intel_display(crtc_state); @@ -536,7 +543,6 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state) trans_vrr_ctl = intel_de_read(display, TRANS_VRR_CTL(display, cpu_transcoder)); - crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE; if (HAS_CMRR(display)) crtc_state->cmrr.enable = (trans_vrr_ctl & VRR_CTL_CMRR_ENABLE); @@ -576,6 +582,14 @@ 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); + + /* + * #TODO: For Both VRR and CMRR the flag I915_MODE_FLAG_VRR is set for mode_flags. + * Since CMRR is currently disabled, set this flag for VRR for now. + * Need to keep this in mind while re-enabling CMRR. + */ if (crtc_state->vrr.enable) crtc_state->mode_flags |= I915_MODE_FLAG_VRR; } -- 2.51.0 From 1f44247dde98b582d7d6ce7d402facc070fa4506 Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:49 +0530 Subject: [PATCH 14/16] drm/i915/vrr: Use crtc_vtotal for vmin MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit To have fixed refresh rate with VRR timing generator the guardband/pipeline full can't be programmed on the fly. So we need to ensure that the values satisfy both the fixed and variable refresh rates. Since we compute these value based on vmin, lets set the vmin to crtc_vtotal for both fixed and variable timings instead of using the current refresh rate based approach. This way the guardband remains sufficient for both cases. v2: Avoid using vblank delay while computing vtotal, as this comes into the picture later. (Ville) Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-7-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 7320eb97991f..e0573e28014b 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -247,17 +247,16 @@ void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) } static -int intel_vrr_compute_vmin(struct intel_connector *connector, - const struct drm_display_mode *adjusted_mode) +int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state) { - const struct drm_display_info *info = &connector->base.display_info; - int vmin; - - vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, - adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq); - vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal); - - return vmin; + /* + * To make fixed rr and vrr work seamless the guardband/pipeline full + * should be set such that it satisfies both the fixed and variable + * timings. + * For this set the vmin as crtc_vtotal. With this we never need to + * change anything to do with the guardband. + */ + return crtc_state->hw.adjusted_mode.crtc_vtotal; } static @@ -304,7 +303,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, if (HAS_LRR(display)) crtc_state->update_lrr = true; - vmin = intel_vrr_compute_vmin(connector, adjusted_mode); + vmin = intel_vrr_compute_vmin(crtc_state); vmax = intel_vrr_compute_vmax(connector, adjusted_mode); if (vmin >= vmax) -- 2.51.0 From bef1e60c7087418eea26f85a19dca7e6360857a9 Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:50 +0530 Subject: [PATCH 15/16] drm/i915/vrr: Prepare for fixed refresh rate timings MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently we always compute the timings as if vrr is enabled. With this approach the state checker becomes complicated when we introduce fixed refresh rate mode with vrr timing generator. To avoid the complications, instead of always computing vrr timings, we compute vrr timings based on uapi.vrr_enable knob. So when the knob is disabled we always compute vmin=flipline=vmax. v2: Use actual timings without any adjustments while preparing for fixed timings in compute_config. (Ville) v3: Avoid setting fixed timings if !vrr_possible(). v4: Move vmin adjustement after all other timings are complete. (Ville) Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä (#v2) Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-8-ankit.k.nautiyal@intel.com --- drivers/gpu/drm/i915/display/intel_vrr.c | 87 ++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index e0573e28014b..622a70e21737 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -246,6 +246,72 @@ void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) crtc_state->mode_flags |= I915_MODE_FLAG_VRR; } +/* + * For fixed refresh rate mode Vmin, Vmax and Flipline all are set to + * Vtotal value. + */ +static +int intel_vrr_fixed_rr_vtotal(const struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + int crtc_vtotal = crtc_state->hw.adjusted_mode.crtc_vtotal; + + if (DISPLAY_VER(display) >= 13) + return crtc_vtotal; + else + return crtc_vtotal - + intel_vrr_real_vblank_delay(crtc_state); +} + +static +int intel_vrr_fixed_rr_vmax(const struct intel_crtc_state *crtc_state) +{ + return intel_vrr_fixed_rr_vtotal(crtc_state); +} + +static +int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + + return intel_vrr_fixed_rr_vtotal(crtc_state) - + intel_vrr_flipline_offset(display); +} + +static +int intel_vrr_fixed_rr_flipline(const struct intel_crtc_state *crtc_state) +{ + return intel_vrr_fixed_rr_vtotal(crtc_state); +} + +static +void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; + + if (!intel_vrr_possible(crtc_state)) + return; + + intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder), + intel_vrr_fixed_rr_vmin(crtc_state) - 1); + intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), + intel_vrr_fixed_rr_vmax(crtc_state) - 1); + intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), + intel_vrr_fixed_rr_flipline(crtc_state) - 1); +} + +static +void intel_vrr_compute_fixed_rr_timings(struct intel_crtc_state *crtc_state) +{ + /* + * For fixed rr, vmin = vmax = flipline. + * vmin is already set to crtc_vtotal set vmax and flipline the same. + */ + crtc_state->vrr.vmax = crtc_state->hw.adjusted_mode.crtc_vtotal; + crtc_state->vrr.flipline = crtc_state->hw.adjusted_mode.crtc_vtotal; +} + static int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state) { @@ -314,6 +380,13 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, crtc_state->vrr.flipline = crtc_state->vrr.vmin; + if (crtc_state->uapi.vrr_enabled) + intel_vrr_compute_vrr_timings(crtc_state); + else if (is_cmrr_frac_required(crtc_state) && is_edp) + intel_vrr_compute_cmrr_timings(crtc_state); + else + intel_vrr_compute_fixed_rr_timings(crtc_state); + /* * flipline determines the min vblank length the hardware will * generate, and on ICL/TGL flipline>=vmin+1, hence we reduce @@ -321,11 +394,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, */ crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display); - if (crtc_state->uapi.vrr_enabled) - intel_vrr_compute_vrr_timings(crtc_state); - else if (is_cmrr_frac_required(crtc_state) && is_edp) - intel_vrr_compute_cmrr_timings(crtc_state); - if (HAS_AS_SDP(display)) { crtc_state->vrr.vsync_start = (crtc_state->hw.adjusted_mode.crtc_vtotal - @@ -496,6 +564,13 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state) if (!crtc_state->vrr.enable) return; + intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder), + crtc_state->vrr.vmin - 1); + intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), + crtc_state->vrr.vmax - 1); + intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), + crtc_state->vrr.flipline - 1); + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), TRANS_PUSH_EN); @@ -523,6 +598,8 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) TRANS_VRR_STATUS(display, cpu_transcoder), VRR_STATUS_VRR_EN_LIVE, 1000); intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); + + intel_vrr_set_fixed_rr_timings(old_crtc_state); } static -- 2.51.0 From 2e921e1d47e627e575ac94eca9db81e374b1e409 Mon Sep 17 00:00:00 2001 From: Ankit Nautiyal Date: Tue, 11 Mar 2025 15:07:51 +0530 Subject: [PATCH 16/16] drm/i915/display: Enable MSA Ignore Timing PAR only when in not fixed_rr mode MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit MSA Ignore Timing PAR enable is set in the DP sink when we enable variable refresh rate. Currently for link training we depend on flipline to decide whether we want to ignore the msa timings. With fixed refresh rate we will still fill the flipline in all cases whether panel supports VRR or not. Change the condition for link training to ignore the msa timings if vrr.in_range. v2: Add more documentation and a #TODO for readout of vrr.in_range. (Ville) Signed-off-by: Ankit Nautiyal Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20250311093751.1329043-9-ankit.k.nautiyal@intel.com --- .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index ded246bbf232..53480914f239 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -727,8 +727,21 @@ void intel_dp_link_training_set_mode(struct intel_dp *intel_dp, int link_rate, b static void intel_dp_update_downspread_ctrl(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { + /* + * Currently, we set the MSA ignore bit based on vrr.in_range. + * We can't really read that out during driver load since we don't have + * the connector information read in yet. So if we do end up doing a + * modeset during initial_commit() we'll clear the MSA ignore bit. + * GOP likely wouldn't have set this bit so after the initial commit, + * if there are no modesets and we enable VRR mode seamlessly + * (without a full modeset), the MSA ignore bit might never get set. + * + * #TODO: Implement readout of vrr.in_range. + * We need fastset support for setting the MSA ignore bit in DPCD, + * especially on the first real commit when clearing the inherited flag. + */ intel_dp_link_training_set_mode(intel_dp, - crtc_state->port_clock, crtc_state->vrr.flipline); + crtc_state->port_clock, crtc_state->vrr.in_range); } void intel_dp_link_training_set_bw(struct intel_dp *intel_dp, -- 2.51.0