On Mon, 10 Mar 2025, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently we loop over all joined pipes and enable/disable the
> shared dplls for each. We don't really have to do that since
> all joined pipes will be using the same dpll. So let's just do
> the enable/disable once for the whole set of joined pipes.
> We can still keep tracking the dpll active set as pipes as long
> as we remember to flip the bits for all the joined pipes on one go.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 16 +++-------------
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 4 ++--
> 2 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3afb85fe8536..0e0e5285ad97 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1658,17 +1658,12 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
> for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i)
> intel_dmc_enable_pipe(display, pipe_crtc->pipe);
>
> intel_encoders_pre_pll_enable(state, crtc);
>
> - for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> - const struct intel_crtc_state *pipe_crtc_state =
> - intel_atomic_get_new_crtc_state(state, pipe_crtc);
> -
> - if (pipe_crtc_state->shared_dpll)
> - intel_enable_shared_dpll(pipe_crtc_state);
> - }
> + if (new_crtc_state->shared_dpll)
> + intel_enable_shared_dpll(new_crtc_state);
Seems like the check for new_crtc_state->shared_dpll could also be moved
inside intel_enable_shared_dpll(), in a separate patch.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> intel_encoders_pre_enable(state, crtc);
>
> for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> const struct intel_crtc_state *pipe_crtc_state =
> @@ -1795,16 +1790,11 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> * Need care with mst->ddi interactions.
> */
> intel_encoders_disable(state, crtc);
> intel_encoders_post_disable(state, crtc);
>
> - for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> - const struct intel_crtc_state *old_pipe_crtc_state =
> - intel_atomic_get_old_crtc_state(state, pipe_crtc);
> -
> - intel_disable_shared_dpll(old_pipe_crtc_state);
> - }
> + intel_disable_shared_dpll(old_crtc_state);
>
> intel_encoders_post_pll_disable(state, crtc);
>
> for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i)
> intel_dmc_disable_pipe(display, pipe_crtc->pipe);
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index c825a507b905..c7b73cd4bc67 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -255,11 +255,11 @@ static void _intel_disable_shared_dpll(struct intel_display *display,
> void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
> {
> struct intel_display *display = to_intel_display(crtc_state);
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct intel_shared_dpll *pll = crtc_state->shared_dpll;
> - unsigned int pipe_mask = BIT(crtc->pipe);
> + unsigned int pipe_mask = intel_crtc_joined_pipe_mask(crtc_state);
> unsigned int old_mask;
>
> if (drm_WARN_ON(display->drm, !pll))
> return;
>
> @@ -301,11 +301,11 @@ void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
> void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
> {
> struct intel_display *display = to_intel_display(crtc_state);
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct intel_shared_dpll *pll = crtc_state->shared_dpll;
> - unsigned int pipe_mask = BIT(crtc->pipe);
> + unsigned int pipe_mask = intel_crtc_joined_pipe_mask(crtc_state);
>
> /* PCH only available on ILK+ */
> if (DISPLAY_VER(display) < 5)
> return;
--
Jani Nikula, Intel
