diff mbox series

[01/11] drm/i915: Check pipe active state in {planes, vrr}_{enabling, disabling}()

Message ID 20231106211915.13406-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Audio fastset, and some fixes | expand

Commit Message

Ville Syrjälä Nov. 6, 2023, 9:19 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

{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@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jani Nikula Nov. 13, 2023, 3:31 p.m. UTC | #1 
On Mon, 06 Nov 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> {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@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  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 ||
diff mbox series

Patch

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 ||