[Intel-gfx] [PATCH 1/4] drm/i915/psr: Only lookup for enabled CRTCs when forcing a fastset

[Maarten Lankhorst]

Op 23-02-2019 om 03:02 schreef Dhinakaran Pandiyan:
> On Fri, 2019-02-22 at 16:33 -0800, Souza, Jose wrote:
>> On Fri, 2019-02-22 at 16:27 -0800, Pandiyan, Dhinakaran wrote:
>>> On Wed, 2019-02-13 at 18:02 -0800, José Roberto de Souza wrote:
>>>> Forcing a specific CRTC to the eDP connector was causing the
>>>> intel_psr_fastset_force() to mark mode_chaged in the wrong and
>>>> disabled CRTC causing no update in the PSR state.
>>>>
>>>> Looks like our internal state track do not clear output_types and
>>>> has_psr in the disabled CRTCs, not sure if this is the expected
>>>> behavior or not but in the mean time this fix the issue.
>>> Is there an IGT that's failing because of this? Looks like the
>>> state
>>> would have got cleared only if the crtc was enabled to drive
>>> another
>>> encoder.
>> When PSR2 is enabled by default tests like 
>> kms_pipe_crc_basic at read-crc-pipe-b are failling even with the patch
>> that disable PSR2 when getting CRC.
> Thanks!
>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>>>> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_psr.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_psr.c
>>>> b/drivers/gpu/drm/i915/intel_psr.c
>>>> index 75c1a5deebf5..08967836b48e 100644
>>>> --- a/drivers/gpu/drm/i915/intel_psr.c
>>>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>>>> @@ -986,7 +986,8 @@ static int intel_psr_fastset_force(struct
>>>> drm_i915_private *dev_priv)
>>>>  
>>>>  		intel_crtc_state = to_intel_crtc_state(crtc_state);
>>>>  
>>>> -		if (intel_crtc_has_type(intel_crtc_state,
>>>> INTEL_OUTPUT_EDP) &&
>>>> +		if (crtc_state->enable &&
>>>> +		    intel_crtc_has_type(intel_crtc_state,
>>>> INTEL_OUTPUT_EDP) &&
>>>>  		    intel_crtc_state->has_psr) {
>
> if (crtc_state->enable && intel_crtc_state->has_psr)
> 	should cover all the cases, no?
>
> And also add a WARN() in case we somehow end up with more than once
> crtc with the above condition being true.


Please use crtc_state->active

crtc_state->enable only tells if the crtc is configured. It doesn't mean thatthe pipe is running.

~Maarten