Re: [PATCH 9/9] drm/i915/display: Add DC3CO disable handling for psr2

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

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


On 05-01-2026 18:31, Jani Nikula wrote:
> On Tue, 09 Dec 2025, Dibin Moolakadan Subrahmanian<dibin.moolakadan.subrahmanian@intel.com> wrote:
>> dc6 should be enabled instead of dc3co after  6 idle frames
>> while in psr2.(re enable part of tgl dc3co handling)
> Please write proper commit messages. I don't understand what this patch
> is supposed to do based on this.

I will add more details to commit message .
On TGL, original code disabled DC3CO after 6 consecutive idle frames and
re-enabled DC6. That logic was removed for the new DC3CO design. This patch
restores the behavior in PSR2, switching from DC3CO to DC6 after 6 idle
frames to maintain correct power management.

>
>> Signed-off-by: Dibin Moolakadan Subrahmanian<dibin.moolakadan.subrahmanian@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_types.h    |  1 +
>>   drivers/gpu/drm/i915/display/intel_psr.c      | 78 ++++++++++++++++++-
>>   2 files changed, 78 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 27f69df7ee9c..6ff53cd58052 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1759,6 +1759,7 @@ struct intel_psr {
>>   	bool panel_replay_enabled;
>>   	u32 dc3co_exitline;
>>   	u32 dc3co_exit_delay;
>> +	struct delayed_work dc3co_work;
>>   	u8 entry_setup_frames;
>>   
>>   	u8 io_wake_lines;
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>> index 18bf45455ea2..4be709d1d324 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> @@ -1157,6 +1157,78 @@ static void psr2_program_idle_frames(struct intel_dp *intel_dp,
>>   		     EDP_PSR2_IDLE_FRAMES(idle_frames));
>>   }
>>   
>> +static void psr2_dc3co_disable(struct intel_dp *intel_dp)
>> +{
>> +	struct intel_display *display = to_intel_display(intel_dp);
>> +	struct i915_power_domains *power_domains = &display->power.domains;
> There's currently one place in intel_psr.c that checks
> power_domains->allowed_dc_mask, and I think even that is too much.
>
> display->power belongs to intel_display_power*.c, and nobody else.
>
> I think you probably need a helper function to ask for this stuff from
> power modules.

Thanks for pointing this out.
I will add a helper function in intel_display_power.c to query this
instead of accessing display->power directly.

>> +
>> +	if ((power_domains->allowed_dc_mask & DC_STATE_EN_UPTO_DC3CO) != DC_STATE_EN_UPTO_DC3CO)
>> +		return;
>> +
>> +	intel_display_power_set_target_dc_state(display, DC_STATE_EN_UPTO_DC6);
>> +	/* Todo restore PSR2 idle frames , ALPM control*/
> 	/* TODO: restore PSR2 idle frames, ALPM control */
>
>> +}
>> +
>> +static void psr2_dc3co_disable_on_exit(struct intel_dp *intel_dp)
>> +{
>> +	struct intel_display *display = to_intel_display(intel_dp);
>> +	struct i915_power_domains *power_domains = &display->power.domains;
>> +
>> +	if ((power_domains->allowed_dc_mask & DC_STATE_EN_UPTO_DC3CO) != DC_STATE_EN_UPTO_DC3CO)
>> +		return;
>> +
>> +	cancel_delayed_work(&intel_dp->psr.dc3co_work);
>> +	intel_dc3co_source_unset(display, DC3CO_SOURCE_PSR2);
>> +}
>> +
>> +static void psr2_dc3co_disable_work(struct work_struct *work)
>> +{
>> +	struct intel_dp *intel_dp =
>> +		container_of(work, typeof(*intel_dp), psr.dc3co_work.work);
>> +
>> +	mutex_lock(&intel_dp->psr.lock);
>> +	/* If delayed work is pending, it is not idle */
>> +	if (delayed_work_pending(&intel_dp->psr.dc3co_work))
>> +		goto unlock;
>> +	/* enable DC6 after idle frames*/
>> +	psr2_dc3co_disable(intel_dp);
>> +
>> +unlock:
>> +	mutex_unlock(&intel_dp->psr.lock);
>> +}
>> +
>> +/*
>> + * When we will be completely rely on PSR2 S/W tracking in future,
>> + * intel_psr_flush() will invalidate and flush the PSR for ORIGIN_FLIP
>> + * event also therefore psr2_dc3co_flush_locked() require to be changed
>> + * accordingly in future.
>> + */
>> +
>> +static void
>> +psr2_dc3co_flush_locked(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
>> +			enum fb_op_origin origin)
>> +{
>> +	struct intel_display *display = to_intel_display(intel_dp);
>> +	struct i915_power_domains *power_domains = &display->power.domains;
>> +
>> +	if (!(power_domains->allowed_dc_mask & DC_STATE_EN_UPTO_DC3CO))
>> +		return;
>> +
>> +	if (!intel_dp->psr.sel_update_enabled ||
>> +	    !intel_dp->psr.active)
>> +		return;
>> +	/*
>> +	 * At every frontbuffer flush flip event modified delay of delayed work,
>> +	 * when delayed work schedules that means display has been idle.
>> +	 */
>> +	if (!(frontbuffer_bits &
>> +	    INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe)))
>> +		return;
>> +
>> +	mod_delayed_work(display->wq.unordered, &intel_dp->psr.dc3co_work,
>> +			 intel_dp->psr.dc3co_exit_delay);
>> +}
>> +
>>   static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>>   					      struct intel_crtc_state *crtc_state)
>>   {
>> @@ -2117,7 +2189,7 @@ static void intel_psr_exit(struct intel_dp *intel_dp)
>>   		intel_de_rmw(display, TRANS_DP2_CTL(intel_dp->psr.transcoder),
>>   			     TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
>>   	} else if (intel_dp->psr.sel_update_enabled) {
>> -
>> +		psr2_dc3co_disable_on_exit(intel_dp);
>>   		val = intel_de_rmw(display,
>>   				   EDP_PSR2_CTL(display, cpu_transcoder),
>>   				   EDP_PSR2_ENABLE, 0);
>> @@ -2259,6 +2331,7 @@ void intel_psr_disable(struct intel_dp *intel_dp,
>>   
>>   	mutex_unlock(&intel_dp->psr.lock);
>>   	cancel_work_sync(&intel_dp->psr.work);
>> +	cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
>>   }
>>   
>>   /**
>> @@ -2289,6 +2362,7 @@ void intel_psr_pause(struct intel_dp *intel_dp)
>>   	mutex_unlock(&psr->lock);
>>   
>>   	cancel_work_sync(&psr->work);
>> +	cancel_delayed_work_sync(&psr->dc3co_work);
>>   }
>>   
>>   /**
>> @@ -3475,6 +3549,7 @@ void intel_psr_flush(struct intel_display *display,
>>   		if (origin == ORIGIN_FLIP ||
>>   		    (origin == ORIGIN_CURSOR_UPDATE &&
>>   		     !intel_dp->psr.psr2_sel_fetch_enabled)) {
>> +			psr2_dc3co_flush_locked(intel_dp, frontbuffer_bits, origin);
>>   			goto unlock;
>>   		}
>>   
>> @@ -3533,6 +3608,7 @@ void intel_psr_init(struct intel_dp *intel_dp)
>>   		intel_dp->psr.link_standby = connector->panel.vbt.psr.full_link;
>>   
>>   	INIT_WORK(&intel_dp->psr.work, intel_psr_work);
>> +	INIT_DELAYED_WORK(&intel_dp->psr.dc3co_work, psr2_dc3co_disable_work);
>>   	mutex_init(&intel_dp->psr.lock);
>>   }

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