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

From: "Kulkarni, Vandita" <vandita.kulkarni@intel.com>
To: "Nikula, Jani" <jani.nikula@intel.com>,
	"Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 3/2] drm/i915/dsc: fix DSC power domains for DSI
Date: Thu, 19 Dec 2019 10:49:58 +0000	[thread overview]
Message-ID: <57510F3E2013164E925CD03ED7512A3B809E9812@BGSMSX108.gar.corp.intel.com> (raw)
In-Reply-To: <20191212134728.18432-1-jani.nikula@intel.com>

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Thursday, December 12, 2019 7:17 PM
> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Souza, Jose <jose.souza@intel.com>; Navare, Manasi D
> <manasi.d.navare@intel.com>; Kulkarni, Vandita
> <vandita.kulkarni@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@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@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@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx