On Tue, 2025-02-18 at 23:18 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Inactive crtcs are supposed to have their crtc_state completely
> cleared. Currently we are clobbering crtc_state->cpu_transcoder
> before determining whether it's actually enabled or not. Don't
> do that.
>
> I want to rework the inherited flag handling for inactive crtcs
> a bit, and having a bogus cpu_transcoder in the crtc state can
> then cause confusing fastset mismatches even when the crtc never
> changes state during the commit.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 28 +++++++++-----------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@xxxxxxxxx>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 8ee8c1df075a..75b184c933e7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3109,25 +3109,24 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> struct intel_display *display = to_intel_display(crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum intel_display_power_domain power_domain;
> + enum transcoder cpu_transcoder = (enum transcoder)crtc->pipe;
> intel_wakeref_t wakeref;
> + bool ret = false;
> u32 tmp;
> - bool ret;
>
> power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> wakeref = intel_display_power_get_if_enabled(display, power_domain);
> if (!wakeref)
> return false;
>
> + tmp = intel_de_read(dev_priv, TRANSCONF(dev_priv, cpu_transcoder));
> + if (!(tmp & TRANSCONF_ENABLE))
> + goto out;
> +
> + pipe_config->cpu_transcoder = cpu_transcoder;
> +
> pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
> pipe_config->sink_format = pipe_config->output_format;
> - pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> -
> - ret = false;
> -
> - tmp = intel_de_read(dev_priv,
> - TRANSCONF(dev_priv, pipe_config->cpu_transcoder));
> - if (!(tmp & TRANSCONF_ENABLE))
> - goto out;
>
> if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv)) {
> @@ -3492,23 +3491,22 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> enum intel_display_power_domain power_domain;
> + enum transcoder cpu_transcoder = (enum transcoder)crtc->pipe;
> intel_wakeref_t wakeref;
> + bool ret = false;
> u32 tmp;
> - bool ret;
>
> power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> wakeref = intel_display_power_get_if_enabled(display, power_domain);
> if (!wakeref)
> return false;
>
> - pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> -
> - ret = false;
> - tmp = intel_de_read(dev_priv,
> - TRANSCONF(dev_priv, pipe_config->cpu_transcoder));
> + tmp = intel_de_read(dev_priv, TRANSCONF(dev_priv, cpu_transcoder));
> if (!(tmp & TRANSCONF_ENABLE))
> goto out;
>
> + pipe_config->cpu_transcoder = cpu_transcoder;
> +
> switch (tmp & TRANSCONF_BPC_MASK) {
> case TRANSCONF_BPC_6:
> pipe_config->pipe_bpp = 18;
