On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote: > On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > Currently we enable FEC even when DSC is no used. While that is > > theoretically valid supposedly there isn't much of a benefit from > > this. But more importantly we do not account for the FEC link > > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations. > > So the code may think we have enough bandwidth when we in fact > > do not. > > > > Cc: Anusha Srivatsa <anusha.sriva...@intel.com> > > Cc: Manasi Navare <manasi.d.nav...@intel.comk> > > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.") > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 326de12c3f44..bbf678561509 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct > > intel_dp *intel_dp, > > int pipe_bpp; > > int ret; > > > > + pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) && > > + intel_dp_supports_fec(intel_dp, pipe_config); > > + > > We could still not enable DSC after this point since it has more checks in > this > function. Even though in that case we would fail the encoder config so wouldnt > matter if we have enabled FEC or not, but its less intutive. > IMHO, the ideal place to set the fec enable is in > intel_dp_compute_link_config() > after the all to dsc_compute_config and set it only if > pipe_config->dsc_params.compression_enable
That would require changing intel_dp_supports_dsc() which I decided wasn't worth the hassle. > > Manasi > > > if (!intel_dp_supports_dsc(intel_dp, pipe_config)) > > return -EINVAL; > > > > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > > return -EINVAL; > > > > - pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) && > > - intel_dp_supports_fec(intel_dp, pipe_config); > > - > > ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state); > > if (ret < 0) > > return ret; > > -- > > 2.19.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx