[Intel-gfx] [PATCH 3/2] drm/i915/dsc: fix DSC power domains for DSI

[Kulkarni, Vandita]

> -----Original Message-----
> From: Jani Nikula <jani.nikula at intel.com>
> Sent: Thursday, December 12, 2019 7:17 PM
> To: Nikula, Jani <jani.nikula at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: Souza, Jose <jose.souza at intel.com>; Navare, Manasi D
> <manasi.d.navare at intel.com>; Kulkarni, Vandita
> <vandita.kulkarni at intel.com>
> Subject: [PATCH 3/2] drm/i915/dsc: fix DSC power domains for DSI
> 
> Fix several issues with DSC power domains that did not take DSI transcoders
> into account:
> 
> - On TGL+ we need to use PW2 for DSC on pipe A, not transcoder A. There
>   is no longer an eDP transcoder, but there are two DSI transcoders
>   which may be connected to pipe A.
> 
> - On TGL+ we need to use the pipe, not transcoder, power domains for DSC
>   on pipes other than A. Again, there are DSI transcoders.
> 
> - On ICL we need to use PW2 for DSC also for DSI transcoders, not just
>   for the eDP transcoder.
> 
> Using is_pipe_dsc() also adds the warning about ICL pipe A DSC, which does
> not exist.
> 
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Manasi Navare <manasi.d.navare at intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vdsc.c | 28 ++++++++++++-----------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 41718f721484..6bab08db5d75 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -469,25 +469,27 @@ int intel_dsc_compute_params(struct
> intel_encoder *encoder,  enum intel_display_power_domain
> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state)  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> 
>  	/*
> -	 * On ICL VDSC/joining for eDP transcoder uses a separate power
> well,
> -	 * PW2. This requires POWER_DOMAIN_TRANSCODER_VDSC_PW2
> power domain.
> -	 * For any other transcoder, VDSC/joining uses the power well
> associated
> -	 * with the pipe/transcoder in use. Hence another reference on the
> -	 * transcoder power domain will suffice.
> +	 * VDSC/joining uses a separate power well, PW2, and requires
> +	 * POWER_DOMAIN_TRANSCODER_VDSC_PW2 power domain in two
> cases:
>  	 *
> -	 * On TGL we have the same mapping, but for transcoder A (the
> special
> -	 * TRANSCODER_EDP is gone).

Comment in drivers/gpu/drm/i915/display/intel_display_power.h where 
POWER_DOMAIN_TRANSCODER_VDSC_PW2 is defined misses DSI transcoder.

Rest all looks good to me.
Reviewed-by: Vandita Kulkarni <vandita.kulkarni at intel.com>

Thanks,
Vandita

> +	 *  - ICL eDP/DSI transcoder
> +	 *  - TGL pipe A
> +	 *
> +	 * For any other pipe, VDSC/joining uses the power well associated
> with
> +	 * the pipe in use. Hence another reference on the pipe power
> domain
> +	 * will suffice. (Except no VDSC/joining on ICL pipe A.)
>  	 */
> -	if (INTEL_GEN(i915) >= 12 && cpu_transcoder == TRANSCODER_A)
> -		return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
> -	else if (cpu_transcoder == TRANSCODER_EDP)
> +	if (INTEL_GEN(i915) >= 12 && pipe == PIPE_A)
>  		return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
> +	else if (is_pipe_dsc(crtc_state))
> +		return POWER_DOMAIN_PIPE(pipe);
>  	else
> -		return POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> +		return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
>  }
> 
>  static void intel_dsc_pps_configure(struct intel_encoder *encoder,
> --
> 2.20.1