On Mon, 06 Nov 2023, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > {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ä <[email protected]>
Reviewed-by: Jani Nikula <[email protected]> > --- > 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
