On Mon, 06 Nov 2023, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> {planes,vrr}_{enabling,disabling}() are supposed to indicate
> whether the specific hardware feature is supposed to be enabling
> or disabling. That can only makes sense if the pipe is active
> overall. So check for that before we go poking at the hardware.
>
> I think we're semi-safe currently on due to:
> - intel_pre_plane_update() doesn't get called when the pipe
> was not-active prior to the commit, but this is actually a bug.
> This saves vrr_disabling(), and vrr_enabling() is called from
> deeper down where we have already checked hw.active.
> - active_planes mirrors the crtc's hw.active
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 9e9c03287869..f24c410cbd8f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -902,12 +902,18 @@ static bool needs_async_flip_vtd_wa(const struct intel_crtc_state *crtc_state)
> static bool planes_enabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> + if (!new_crtc_state->hw.active)
> + return false;
> +
> return is_enabling(active_planes, old_crtc_state, new_crtc_state);
> }
>
> static bool planes_disabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> + if (!old_crtc_state->hw.active)
> + return false;
> +
> return is_disabling(active_planes, old_crtc_state, new_crtc_state);
> }
>
> @@ -924,6 +930,9 @@ static bool vrr_params_changed(const struct intel_crtc_state *old_crtc_state,
> static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> + if (!new_crtc_state->hw.active)
> + return false;
> +
> return is_enabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> (new_crtc_state->vrr.enable &&
> (new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
> @@ -933,6 +942,9 @@ static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
> const struct intel_crtc_state *new_crtc_state)
> {
> + if (!old_crtc_state->hw.active)
> + return false;
> +
> return is_disabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> (old_crtc_state->vrr.enable &&
> (new_crtc_state->update_m_n || new_crtc_state->update_lrr ||
--
Jani Nikula, Intel
