[PATCH 02/12] drm/i915/dp: Return early if DSC not supported

[Kandpal, Suraj]

> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
> Sent: Tuesday, December 3, 2024 2:02 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: intel-xe at lists.freedesktop.org; jani.nikula at linux.intel.com; Deak, Imre
> <imre.deak at intel.com>
> Subject: Re: [PATCH 02/12] drm/i915/dp: Return early if DSC not supported
> 
> 
> On 11/27/2024 11:13 AM, Kandpal, Suraj wrote:
> >
> >> -----Original Message-----
> >> From: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
> >> Sent: Wednesday, November 20, 2024 4:08 PM
> >> To: intel-gfx at lists.freedesktop.org
> >> Cc: intel-xe at lists.freedesktop.org; Kandpal, Suraj
> >> <suraj.kandpal at intel.com>; jani.nikula at linux.intel.com; Deak, Imre
> >> <imre.deak at intel.com>
> >> Subject: [PATCH 02/12] drm/i915/dp: Return early if DSC not supported
> >>
> >> Check for DSC support before computing link config with DSC.
> >> For DP MST we are already doing the same.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dp.c | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index db9ddbcdd159..dee15a05e7fd 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -2378,9 +2378,6 @@ int intel_dp_dsc_compute_config(struct intel_dp
> >> *intel_dp,
> >>   		 intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
> >>   		 !intel_dp_is_uhbr(pipe_config));
> >>
> >> -	if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
> >> -		return -EINVAL;
> >> -
> >>   	if (!intel_dp_dsc_supports_format(connector, pipe_config-
> >>> output_format))
> >>   		return -EINVAL;
> >>
> >> @@ -2643,6 +2640,9 @@ intel_dp_compute_link_config(struct
> >> intel_encoder *encoder,
> >>   			    str_yes_no(ret), str_yes_no(joiner_needs_dsc),
> >>   			    str_yes_no(intel_dp->force_dsc_en));
> >>
> >> +		if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
> >> +			return -EINVAL;
> >> +
> > Mostly looks good to me but I was thinking what if we made
> > intel_dp_supports_dsc one of the conditions that Determines if dsc is
> needed or not.
> 
> 
> Thanks Suraj for looking into the series once again.
> 
> I think that mixing intel_dp_supports_dsc with dsc_needed will complicate
> the check.
> 
> Currently dsc_is_needed is set: if dsc is forced or if its needed for joiner case
> or if its needed because bandwidth is not sufficient for the given mode.
> 
> If dsc is not needed, we dont need to check DSC support.
> 
> If DSC is indeed required, first logical thing to do should be to check if DSC is
> supported.
> 
> 

Okay then it LGTM
Reviewed-by: Suraj Kandpal <suraj.kandpal at intel.com>

> Regards,
> 
> Ankit
> 
> >
> > Regards,
> > Suraj Kandpal
> >
> >>   		if (!intel_dp_compute_config_limits(intel_dp, pipe_config,
> >>
> >> respect_downstream_limits,
> >>   						    true,
> >> --
> >> 2.45.2