Re: [PATCH 07/11] drm/i915: add update function to disable/enable-back PSR

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jul 15, 2013 at 05:21:12PM -0300, Rodrigo Vivi wrote:
> On Mon, Jul 15, 2013 at 11:00 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, Jul 11, 2013 at 06:45:01PM -0300, Rodrigo Vivi wrote:
> >> @@ -1602,6 +1611,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
> >>               DRM_ERROR("Timed out waiting for PSR Idle State\n");
> >>  }
> >>
> >> +void intel_edp_psr_update(struct drm_device *dev)
> >> +{
> >> +     struct intel_encoder *encoder;
> >> +     struct intel_dp *intel_dp = NULL;
> >> +
> >> +     list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
> >> +             if (encoder->type == INTEL_OUTPUT_EDP) {
> >
> > How many eDP are you planning to allow on the system? We already have
> > precedence for the presumption of a single (integrated) panel on a device,
> > maybe we can add the logic there (i.e. stash a back pointer in this case)?
> 
> That is a good question... I asked it myself many times when I was
> trying to get intel_dp with edp from dev...
> For the first version I just run the loop getting any intel_dp with
> edp since we have this assumption of only one edp,
> but then I thought about that convertibles with 2 panels and since in
> hsw we can have edp on port D I decided to let the implementation
> more generic as possible although I know we won't have this case... at
> least not any time soon.

The way I nowadays solve such a conundrum is to shovel a bit of metadata
(like psr_capable_sink) into pipe_config and let encoders fill it out
appropriately in their ->compute_config functions. Most of the "walk over
all encoders and noodle int their innards" have disappeared through
that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch