On Wed, 2013-10-16 at 08:08 -0700, Jesse Barnes wrote: > On Wed, 16 Oct 2013 14:10:13 +0300 > Imre Deak <imre.deak@xxxxxxxxx> wrote: > > > On Tue, 2013-10-15 at 13:40 -0700, Jesse Barnes wrote: > > > On Tue, 15 Oct 2013 16:54:00 -0300 > > > Paulo Zanoni <przanoni@xxxxxxxxx> wrote: > > > [...] > > > No that's taken into account here. In __intel_set_mode we take a > > > private ref on the appropriate power well so that we'll preserve state > > > until we do the first crtc_enable. From then on, the ref is tracked > > > there and we drop the private one in __intel_set_mode > > > > > > > > + if (crtc->active) > > > > > + intel_display_power_get(dev, > > > > > + POWER_DOMAIN_PIPE(crtc->pipe)); > > > > > + > > > > > > > > What about the panel fitter power domains? Sometimes the panel fitter > > > > is the thing that makes you require a power well, even though you're > > > > on a pipe that doesn't need it. > > > > > > > > And on Haswell you also have to take into account > > > > TRANSCODER_EDP+PIPE_A versus TRANSCODER_A+PIPE_A, where the first > > > > doesn't need the power well but the second needs it. > > > > > > Yeah I'm still not sure how to handle this in generic code. Maybe the > > > power well mapping function Imre added will be enough, but it > > > definitely gets tricky when we look at all the different platforms we > > > have to (and will have to) handle. > > > > Isn't the power domain abstraction a neat idea exactly for the above > > case? Generic code just asks for the domain it needs and doesn't care > > how it maps to power wells on the given platform. So for transcoder_edp > > +pipe_a it'd end up asking for POWER_DOMAIN_PIPE_A and > > POWER_DOMAIN_TRANSCODER_EDP, both of which is a nop on HSW, and for the > > other case POWER_DOMAIN_PIPE_A and POWER_DOMAIN_TRANSCODER_A which would > > enable the power well. You also have the POWER_DOMAIN_PIPE, > > POWER_DOMAIN_TRANSCODER, POWER_DOMAIN_PIPE_PANEL_FITTER helpers already. > > Yeah I think it can work. I missed your function that takes a crtc > though as well, so we don't end up polluting the generic functions with > TRANSCODER references that don't exist on the Atom platforms for > example. That's the main thing I'm worried about, since as we get more > and more wells I think it'll get easier to get it wrong in the generic > code, if we have to use all the required domains for all platforms > there. Afaics, on VLV for example we'd ask for pipe A/B and transcoder A/B power domains, which is still correct. It's true that there the pipe-transcoder connection is fixed, and so we'll always ask for the same pipe/transcoder power domain pair, but I think it's still ok conceptually. So atm the power domains as defined are platform independent, which is great. If we can't avoid adding a platform specific ones in the future, we could still handle those in platform specific code. --Imre
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
