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

[Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>]

[-- 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 --]