Re: [PATCH v2] drm/i915/psr: Execute the default PSR code path when setting i915_edp_psr_debug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2019-01-31 at 10:34 +0100, Maarten Lankhorst wrote:
> Op 31-01-2019 om 01:58 schreef José Roberto de Souza:
> > Changing the i915_edp_psr_debug was enabling, disabling or
> > switching
> > PSR version by directly calling intel_psr_disable_locked() and
> > intel_psr_enable_locked(), what is not the default PSR path that
> > will
> > be executed by real users.
> > 
> > So lets force a fastset in the PSR CRTC to trigger a pipe update
> > and
> > stress the default code path.
> > 
> > Recently a bug was found when switching from PSR2 to PSR1 while
> > enable_psr kernel parameter was set to the default parameter, this
> > changes fix it and also fixes the bug linked bellow were DRRS was
> > left enabled together with PSR when enabling PSR from debugfs.
> > 
> > v2: Handling missing case: disabled to PSR1
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108341
> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> > 
> > Should I add IGT tests to test every state switch combination?
> 
> Should probably be done for DRRS as well. We should be able to stop
> having to set has_drrs and has_psr unconditionally then. :)
> Could be a separate followup patch.

I have DRRS/PSR bugs assigned to me, I will definitely fix that in the
future.

> 
> The complete duplication of the whole atomic state is overkill and
> should be avoided, just use 
> if (!intel_crtc_has_type(to_intel_crtc_state(crtc->state),
> INTEL_OUTPUT_EDP)
> 	continue;
> 
> crtc_state = drm_atomic_get_old_crtc_state(...)
> ....
> 
> And then do a normal commit. We will add all planes and connectors as
> needed.

Thanks for the suggestion, just sent the new version with this.

> 
> With that fixed:
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> 

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux