[v2] drm/i915: Fix inconsistance between pfit.enable and scaler freeing
diff mbox series

Message ID 20200124144216.4045-1-stanislav.lisovskiy@intel.com
State New
Headers show
Series
  • [v2] drm/i915: Fix inconsistance between pfit.enable and scaler freeing
Related show

Commit Message

Stanislav Lisovskiy Jan. 24, 2020, 2:42 p.m. UTC
Despite that during hw readout we seem to have scalers assigned
to pipes, then call atomic_setup_scalers, at the commit stage in
skl_update_scaler there is a check, that if we have fb src and
dest of same size, we stage freeing of that scaler.

However we don't update pfit.enabled flag then, which makes
the state inconsistent, which in turn triggers a WARN_ON
in skl_pfit_enable, because we have pfit enabled,
but no assigned scaler.

To me this looks weird that we kind of do the decision
to use or not use the scaler at skl_update_scaler stage
but not in intel_atomic_setup_scalers, moreover
not updating the whole state consistently.

This fix is to not free the scaler if we have pfit.enabled
flag set, so that the state is now consistent
and the warnings are gone.

Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/577
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ville Syrjälä Jan. 24, 2020, 2:55 p.m. UTC | #1
On Fri, Jan 24, 2020 at 04:42:16PM +0200, Stanislav Lisovskiy wrote:
> Despite that during hw readout we seem to have scalers assigned
> to pipes, then call atomic_setup_scalers, at the commit stage in
> skl_update_scaler there is a check, that if we have fb src and
> dest of same size, we stage freeing of that scaler.
> 
> However we don't update pfit.enabled flag then, which makes
> the state inconsistent, which in turn triggers a WARN_ON
> in skl_pfit_enable, because we have pfit enabled,
> but no assigned scaler.
> 
> To me this looks weird that we kind of do the decision
> to use or not use the scaler at skl_update_scaler stage
> but not in intel_atomic_setup_scalers, moreover
> not updating the whole state consistently.
> 
> This fix is to not free the scaler if we have pfit.enabled
> flag set, so that the state is now consistent
> and the warnings are gone.
> 
> Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/577
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5768cfcf71c4..23221b8d244d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5953,7 +5953,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  	 * the 90/270 degree plane rotation cases (to match the
>  	 * GTT mapping), hence no need to account for rotation here.
>  	 */
> -	if (src_w != dst_w || src_h != dst_h)
> +	if (src_w != dst_w || src_h != dst_h || crtc_state->pch_pfit.enabled)

This is called for both pfit and planes. So you need to put
that pfit check into the pfit specific function.

>  		need_scaler = true;
>  
>  	/*
> -- 
> 2.24.1.485.gad05a3d8e5

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5768cfcf71c4..23221b8d244d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5953,7 +5953,7 @@  skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 * the 90/270 degree plane rotation cases (to match the
 	 * GTT mapping), hence no need to account for rotation here.
 	 */
-	if (src_w != dst_w || src_h != dst_h)
+	if (src_w != dst_w || src_h != dst_h || crtc_state->pch_pfit.enabled)
 		need_scaler = true;
 
 	/*