RE: [PATCH 11/22] drm/i915/mst: Check intel_dp_joiner_needs_dsc()

From: "Shankar, Uma" <uma.shankar@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 11/22] drm/i915/mst: Check intel_dp_joiner_needs_dsc()
Date: Mon, 1 Apr 2024 09:28:15 +0000	[thread overview]
Message-ID: <DM4PR11MB636030470E994C5470EF9499F43F2@DM4PR11MB6360.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240329011254.24160-12-ville.syrjala@linux.intel.com>

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Friday, March 29, 2024 6:43 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 11/22] drm/i915/mst: Check intel_dp_joiner_needs_dsc()
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_dp_mst_compute_config() is missing the "does the joiner need DSC?" check
> despite claiming to have a lot of other joiner/dsc stuff in there (albeit disabled).
> Replicate the logic from the SST side.
> 
> TODO: refactor all this duplicated code!

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 6da031f9724d..1cf6241a7d53 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -532,7 +532,7 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
>  	const struct drm_display_mode *adjusted_mode =
>  		&pipe_config->hw.adjusted_mode;
>  	struct link_config_limits limits;
> -	bool dsc_needed;
> +	bool dsc_needed, joiner_needs_dsc;
>  	int ret = 0;
> 
>  	if (pipe_config->fec_enable &&
> @@ -546,7 +546,9 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_pch_encoder = false;
> 
> -	dsc_needed = intel_dp->force_dsc_en ||
> +	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv,
> +pipe_config->bigjoiner_pipes);
> +
> +	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
>  		     !intel_dp_mst_compute_config_limits(intel_dp,
>  							 connector,
>  							 pipe_config,
> @@ -566,8 +568,8 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
> 
>  	/* enable compression if the mode doesn't fit available BW */
>  	if (dsc_needed) {
> -		drm_dbg_kms(&dev_priv->drm, "Try DSC (fallback=%s,
> force=%s)\n",
> -			    str_yes_no(ret),
> +		drm_dbg_kms(&dev_priv->drm, "Try DSC (fallback=%s,
> joiner=%s, force=%s)\n",
> +			    str_yes_no(ret), str_yes_no(joiner_needs_dsc),
>  			    str_yes_no(intel_dp->force_dsc_en));
> 
>  		if (!intel_dp_mst_dsc_source_support(pipe_config))
> --
> 2.43.2