Hi,
On 25 November 2015 at 12:21, Ander Conselvan De Oliveira
<conselvan2@xxxxxxxxx> wrote:
> On Thu, 2015-11-19 at 16:07 +0100, Maarten Lankhorst wrote:
>> @@ -4754,6 +4751,9 @@ static void intel_post_plane_update(struct intel_crtc
>> *crtc)
>> if (atomic->post_enable_primary)
>> intel_post_enable_primary(&crtc->base);
>>
>> + if (needs_modeset(&pipe_config->base))
>> + intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>> +
>> memset(atomic, 0, sizeof(*atomic));
>> }
>>
>> @@ -13375,8 +13409,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>> for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> bool modeset = needs_modeset(crtc->state);
>> - bool update_pipe = !modeset &&
>> - to_intel_crtc_state(crtc->state)->update_pipe;
>> + struct intel_crtc_state *pipe_config =
>> + to_intel_crtc_state(crtc->state);
>> + bool update_pipe = !modeset && pipe_config->update_pipe;
>> unsigned long put_domains = 0;
>>
>> if (modeset)
>> @@ -13401,18 +13436,21 @@ static int intel_atomic_commit(struct drm_device
>> *dev,
>> (crtc->state->planes_changed || update_pipe))
>> drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>>
>> + if (pipe_config->base.active &&
>> + (pipe_config->wm_changed || pipe_config->disable_cxsr ||
>> + pipe_config->fb_changed))
>> + crtc_vblank_mask |= 1 << i;
>> +
>> if (put_domains)
>> modeset_put_power_domains(dev_priv, put_domains);
>> -
>> - intel_post_plane_update(intel_crtc);
>> -
>> - if (modeset)
>> - intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>
> Would it be too evil to move the power domain get/put out of the loop? If I
> understand correctly, intel_state->modeset already tells us if there is any crtc
> that needs a modeset, so we could just protect the calls with that. I'm not sure
> what is the impact, but at least we could avoid having the get in
> intel_atomic_commit() and the put in intel_post_plane_enable().
Even better would be if POWER_DOMAIN_MODESET was a part of
put_domains, and all domains were put after completion. I suggested
this during review of Patrik's series, but that seems to have got
lost. :(
Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
