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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 20 Jan 2026, Jani Nikula <jani.nikula@linux.intel.com> 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.

Cc: Imre

>
> 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
>> +	 */
>>  
>>  	/* 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);
>>  }

-- 
Jani Nikula, Intel