> -----Original Message----- > From: Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx> > Sent: Tuesday, December 3, 2024 2:02 PM > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; jani.nikula@xxxxxxxxxxxxxxx; Deak, Imre > <imre.deak@xxxxxxxxx> > 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@xxxxxxxxx> > >> Sent: Wednesday, November 20, 2024 4:08 PM > >> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; Kandpal, Suraj > >> <suraj.kandpal@xxxxxxxxx>; jani.nikula@xxxxxxxxxxxxxxx; Deak, Imre > >> <imre.deak@xxxxxxxxx> > >> 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@xxxxxxxxx> > >> --- > >> 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@xxxxxxxxx> > Regards, > > Ankit > > > > > Regards, > > Suraj Kandpal > > > >> if (!intel_dp_compute_config_limits(intel_dp, pipe_config, > >> > >> respect_downstream_limits, > >> true, > >> -- > >> 2.45.2
