2013/10/18 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>:
> On Wed, 16 Oct 2013 17:25:52 +0300
> Imre Deak <imre.deak@xxxxxxxxx> wrote:
>
>> So far the modeset code enabled all power domains if it needed any. It
>> wasn't a problem since HW generations so far only had one always-on
>> power well and one dynamic power well that can be enabled/disabled. For
>> domains powered by always-on power wells (panel fitter on pipe A and the
>> eDP transcoder) we didn't do anything, for all other domains we just
>> enabled the single dynamic power well.
>>
>> Future HW generations will change this, as they add multiple dynamic
>> power wells. Support for these will be added later, this patch prepares
>> for those by making sure we only enable the required domains.
>>
>> Note that after this change on HSW we'll enable all power domains even
>> if it was the domain for the panel fitter on pipe A or the eDP
>> transcoder. This isn't a problem since the power domain framework
>> already checks if the domain is on an always-on power well and doesn't
>> do anything in this case.
>>
>> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
I updated drm-intel-nightly and now when I boot Haswell with eDP-only,
the power well is enabled, where it should be disabled. Reverting this
patch fixes the problem for me.
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++----
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> 2 files changed, 42 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 8e734f2..e2a4f3b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6561,21 +6561,57 @@ static void hsw_package_c8_gpu_busy(struct drm_i915_private *dev_priv)
>> }
>> }
>>
>> +#define for_each_power_domain(domain, mask) \
>> + for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
>> + if ((1 << (domain)) & (mask))
>> +
>> +static unsigned long get_pipe_power_domains(struct drm_device *dev,
>> + enum pipe pipe, bool pfit_enabled)
>> +{
>> + unsigned long mask;
>> + enum transcoder transcoder;
>> +
>> + transcoder = intel_pipe_to_cpu_transcoder(dev->dev_private, pipe);
>> +
>> + mask = BIT(POWER_DOMAIN_PIPE(pipe));
>> + mask |= BIT(POWER_DOMAIN_TRANSCODER(transcoder));
>> + if (pfit_enabled)
>> + mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe));
>> +
>> + return mask;
>> +}
>> +
>> static void modeset_update_power_wells(struct drm_device *dev)
>> {
>> - bool enable = false;
>> + unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
>> struct intel_crtc *crtc;
>>
>> + /*
>> + * First get all needed power domains, then put all unneeded, to avoid
>> + * any unnecessary toggling of the power wells.
>> + */
>> list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
>> + enum intel_display_power_domain domain;
>> +
>> if (!crtc->base.enabled)
>> continue;
>>
>> - if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.enabled ||
>> - crtc->config.cpu_transcoder != TRANSCODER_EDP)
>> - enable = true;
>> + pipe_domains[crtc->pipe] = get_pipe_power_domains(dev,
>> + crtc->pipe,
>> + crtc->config.pch_pfit.enabled);
>> +
>> + for_each_power_domain(domain, pipe_domains[crtc->pipe])
>> + intel_display_power_get(dev, domain);
>> }
>>
>> - intel_set_power_well(dev, enable);
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
>> + enum intel_display_power_domain domain;
>> +
>> + for_each_power_domain(domain, crtc->enabled_power_domains)
>> + intel_display_power_put(dev, domain);
>> +
>> + crtc->enabled_power_domains = pipe_domains[crtc->pipe];
>> + }
>> }
>>
>> static void haswell_modeset_global_resources(struct drm_device *dev)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 189257d..63a5bfd 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -320,6 +320,7 @@ struct intel_crtc {
>> * some outputs connected to this crtc.
>> */
>> bool active;
>> + unsigned long enabled_power_domains;
>> bool eld_vld;
>> bool primary_enabled; /* is the primary plane (partially) visible? */
>> bool lowfreq_avail;
>
> Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>
> Hope we can get rid of the set_power_well() function soon...
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
