Re: [PATCH v3 1/3] drm/i915/pps: Enable panel power earlier

Dibin Moolakadan Subrahmanian
2026-01-21T06:13:14Z

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On 20-01-2026 19:27, Jani Nikula wrote:
> On Fri, 16 Jan 2026, Suraj Kandpal<suraj.kandpal@intel.com> wrote:
>> From: Mika Kahola<mika.kahola@intel.com>
>>
>> Move intel_pps_on() to intel_dpll_mgr PLL enabling
>> .enable function hook to enable panel power earlier.
>> We need to do this to make sure we are following the
>> modeset sequences of Bspec. This had changed when we
>> moved the PLL PHY enablement for CX0 from .enable_clock
>> to dpll.enable hook
> So I really hate this.
>
> Yeah, maybe it follows the spec now, but what connection does the DPLL
> manager have with the panel power sequencing?
>
> Absolutely nothing.
>
> The DPLL manager has no business calling PPS functions.
>
> Currently only the g4x and DDI encoder code does PPS power calls, and
> they're the only ones who should manage PPS.
>
>> Signed-off-by: Mika Kahola<mika.kahola@intel.com>
>> Signed-off-by: Suraj Kandpal<suraj.kandpal@intel.com>
>> ---
>>
>> v2 -> v3:
>> - Rather than splitting the PHY enablement sequence, enable PPS
>> earlier (Imre)
> Please point me at the review comment. I couldn't find anything that
> would suggest moving the PPS calls to the DPLL manager.
>
> Please let's not do this.
>
> BR,
> Jani.
>
>
>>
>> drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++++--
>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 5 +++++
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index cb91d07cdaa6..1784fa687c03 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2653,8 +2653,10 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> /* 3. Select Thunderbolt */
>> mtl_port_buf_ctl_io_selection(encoder);
>>
>> - /* 4. Enable Panel Power if PPS is required */
>> - intel_pps_on(intel_dp);
>> + /*
>> + * 4. Enable Panel Power if PPS is required
>> + * moved to intel_dpll_mgr .enable hook
>> + */

Moving pps on alone wont help here,
as new sequence will be 6 -> 4 -> 5.

Regards,
Dibin

>>
>> /* 5. Enable the port PLL */
>> intel_ddi_enable_clock(encoder, crtc_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> index 9aa84a430f09..b5655c734c53 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> @@ -40,6 +40,7 @@
>> #include "intel_hti.h"
>> #include "intel_mg_phy_regs.h"
>> #include "intel_pch_refclk.h"
>> +#include "intel_pps.h"
>> #include "intel_step.h"
>> #include "intel_tc.h"
>>
>> @@ -4401,6 +4402,10 @@ static void mtl_pll_enable(struct intel_display *display,
>> if (drm_WARN_ON(display->drm, !encoder))
>> return;
>>
>> + /* Enable Panel Power if PPS is required */
>> + if (intel_encoder_is_dp(encoder))
>> + intel_pps_on(enc_to_intel_dp(encoder));
>> +
>> intel_mtl_pll_enable(encoder, pll, dpll_hw_state);
>> }

[-- Attachment #2: Type: text/html, Size: 4228 bytes --]