[Intel-gfx] [PATCH 00/31] IPS/DRRS/PSR rework with PSR enabled by default

[Vivi, Rodrigo]

On Mon, 2015-11-09 at 11:47 +0000, Daniel Stone wrote:
> Hi Rodrigo,
> 
> On 5 November 2015 at 18:49, Rodrigo Vivi <rodrigo.vivi at intel.com> 
> wrote:
> > So I'm confident we can enable PSR back by default now.
> > 
> > All comments, ideas, suggestions and even bikesheddings are pretty 
> > welcome.
> 
> You did ask for it ...

Thank you! I really appreciate your comments.

> 
> I've been looking at pulling this on top of Maarten's tree, and

I'm afraid I didn't followed completely your idea and maybe because I
don't know the code you are referring to. What tree specifically are
you talking about?

> currently my overriding wish is that, rather than the checks 
> sprinkled
> all over various state-change functions,

Ok, so you are suggesting we totally remove the pipe config variables
right? I haven't considered this possibility since I believed some
people would prefer to let the variables there. And also for PSRxDRRS
conflict this would be safier since DRRS cannot be enabled if PSR is
going to be enabled. With your solution we need to pay attention and
never let someone invert PSR and DRRS orders.


>  we instead had:
> static bool intel_ips_should_enable(struct intel_crtc_state 
> *crtc_state)
> 
> In the pre-atomic commit path, this could look like:
>     bool ips = intel_ips_should_enable(crtc_state);
>     if (ips && !intel_crtc->ips_enabled)
>         intel_ips_enable(intel_crtc);
>     else if (!ips && intel_crtc->ips_enabled)
>         intel_ips_disable(intel_crtc);
> 
> Post-atomic, this would be:
>     intel_flip_work->enable_ips = 
> intel_ips_should_enable(crtc_state);
> and actually doing the enable/disable in the work handler.
> 
> Having one place to inspect the state overall seems better, e.g. in
> the case where we disable the primary plane but retain an overlay
> plane on a CRTC, we keep IPS enabled. However, it doesn't seem like
> there's anything to handle the case where we then disable that 
> overlay
> plane, where with no planes enabled at all, IPS should be disabled.
> 
> Cheers,
> Daniel

Thanks,
Rodrigo.