| Message ID | 20181031004517.17250-4-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Forward Error Correction | expand |
On Tue, Oct 30, 2018 at 05:45:13PM -0700, Anusha Srivatsa wrote: > For DP 1.4 and above, Display Stream compression can be > enabled only if Forward Error Correctin can be performed. > > Check if the sink supports FEC using the helper. > > v2: Mention External DP where ever FEC is mentioned > in the code.Check return status of dpcd reads. (Gaurav) > - Do regular mode check even if FEC is not supported. (manasi) > > v3: Do not perform any dpcd writes in the atomic > check phase. (DK, Manasi) > > v4: Use debug level logging for scenario where sink does > not support a feature. (DK) > > v5: Correct commit message. rebase. > > v6: pass single field instead of an array for helper > function. (manasi) > > Cc: Gaurav K Singh <gaurav.k.singh@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8ae7cf3d4ee1..a344be555dd6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -680,7 +680,7 @@ intel_dp_mode_valid(struct drm_connector *connector, > dsc_slice_count = > drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, > true); > - } else { > + } else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) { > dsc_max_output_bpp = > intel_dp_dsc_get_output_bpp(max_link_clock, > max_lanes, > @@ -690,7 +690,8 @@ intel_dp_mode_valid(struct drm_connector *connector, > intel_dp_dsc_get_slice_count(intel_dp, > target_clock, > mode->hdisplay); > - } > + } else > + DRM_DEBUG_KMS("Sink device does not Support FEC\n"); Please no debug prints in the mode_valid() that would spam a lot. Not enough context in the patch for me to see what happens if FEC isn't supported. Do we just check against the uncompressed limits? > } > > if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) || > @@ -2063,6 +2064,13 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp, > if (pipe == PIPE_A && !intel_dp_is_edp(intel_dp)) > return false; > > + /* DSC not supported if external DP sink does not support FEC */ > + if (!intel_dp_is_edp(intel_dp) && > + !drm_dp_sink_supports_fec(intel_dp->fec_capable)) { Why aren't we checking the source capabilities here as well? > + DRM_DEBUG_KMS("Sink does not support Forward Error Correction, disabling Display Compression\n"); "disabling Display Compression" is not true. We're rejecting the entire thing. > + return false; > + } > + > /* DSC not supported for DSC sink BPC < 8 */ > if (limits->max_bpp < 3 * DP_DSC_MIN_SUPPORTED_BPC) { > DRM_DEBUG_KMS("No DSC support for less than 8bpc\n"); > -- > 2.17.1
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Wednesday, October 31, 2018 2:01 PM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Singh, Gaurav K <gaurav.k.singh@intel.com>; >Jani Nikula <jani.nikula@linux.intel.com>; Navare, Manasi D ><manasi.d.navare@intel.com>; Pandiyan, Dhinakaran ><dhinakaran.pandiyan@intel.com> >Subject: Re: [v4 3/7] i915/dp/fec: Check for FEC Support > >On Tue, Oct 30, 2018 at 05:45:13PM -0700, Anusha Srivatsa wrote: >> For DP 1.4 and above, Display Stream compression can be enabled only >> if Forward Error Correctin can be performed. >> >> Check if the sink supports FEC using the helper. >> >> v2: Mention External DP where ever FEC is mentioned in the code.Check >> return status of dpcd reads. (Gaurav) >> - Do regular mode check even if FEC is not supported. (manasi) >> >> v3: Do not perform any dpcd writes in the atomic check phase. (DK, >> Manasi) >> >> v4: Use debug level logging for scenario where sink does not support a >> feature. (DK) >> >> v5: Correct commit message. rebase. >> >> v6: pass single field instead of an array for helper function. >> (manasi) >> >> Cc: Gaurav K Singh <gaurav.k.singh@intel.com> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Manasi Navare <manasi.d.navare@intel.com> >> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >> b/drivers/gpu/drm/i915/intel_dp.c index 8ae7cf3d4ee1..a344be555dd6 >> 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -680,7 +680,7 @@ intel_dp_mode_valid(struct drm_connector >*connector, >> dsc_slice_count = >> drm_dp_dsc_sink_max_slice_count(intel_dp- >>dsc_dpcd, >> true); >> - } else { >> + } else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) { >> dsc_max_output_bpp = >> intel_dp_dsc_get_output_bpp(max_link_clock, >> max_lanes, >> @@ -690,7 +690,8 @@ intel_dp_mode_valid(struct drm_connector >*connector, >> intel_dp_dsc_get_slice_count(intel_dp, >> target_clock, >> mode->hdisplay); >> - } >> + } else >> + DRM_DEBUG_KMS("Sink device does not Support >FEC\n"); > >Please no debug prints in the mode_valid() that would spam a lot. > >Not enough context in the patch for me to see what happens if FEC isn't >supported. Do we just check against the uncompressed limits? In cases where we cannot do FEC for DP cases, we should be disabling DSC. Combining the checking of FEC support (this patch) with the patch 4 of the series which checks for state and disables DSC suitably. Anusha >> } >> >> if ((mode_rate > max_rate && !(dsc_max_output_bpp && >> dsc_slice_count)) || @@ -2063,6 +2064,13 @@ static bool >intel_dp_dsc_compute_config(struct intel_dp *intel_dp, >> if (pipe == PIPE_A && !intel_dp_is_edp(intel_dp)) >> return false; >> >> + /* DSC not supported if external DP sink does not support FEC */ >> + if (!intel_dp_is_edp(intel_dp) && >> + !drm_dp_sink_supports_fec(intel_dp->fec_capable)) { > >Why aren't we checking the source capabilities here as well? > >> + DRM_DEBUG_KMS("Sink does not support Forward Error >Correction, >> +disabling Display Compression\n"); > >"disabling Display Compression" is not true. We're rejecting the entire thing. > >> + return false; >> + } >> + >> /* DSC not supported for DSC sink BPC < 8 */ >> if (limits->max_bpp < 3 * DP_DSC_MIN_SUPPORTED_BPC) { >> DRM_DEBUG_KMS("No DSC support for less than 8bpc\n"); >> -- >> 2.17.1 > >-- >Ville Syrjälä >Intel
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8ae7cf3d4ee1..a344be555dd6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -680,7 +680,7 @@ intel_dp_mode_valid(struct drm_connector *connector, dsc_slice_count = drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, true); - } else { + } else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) { dsc_max_output_bpp = intel_dp_dsc_get_output_bpp(max_link_clock, max_lanes, @@ -690,7 +690,8 @@ intel_dp_mode_valid(struct drm_connector *connector, intel_dp_dsc_get_slice_count(intel_dp, target_clock, mode->hdisplay); - } + } else + DRM_DEBUG_KMS("Sink device does not Support FEC\n"); } if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) || @@ -2063,6 +2064,13 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp, if (pipe == PIPE_A && !intel_dp_is_edp(intel_dp)) return false; + /* DSC not supported if external DP sink does not support FEC */ + if (!intel_dp_is_edp(intel_dp) && + !drm_dp_sink_supports_fec(intel_dp->fec_capable)) { + DRM_DEBUG_KMS("Sink does not support Forward Error Correction, disabling Display Compression\n"); + return false; + } + /* DSC not supported for DSC sink BPC < 8 */ if (limits->max_bpp < 3 * DP_DSC_MIN_SUPPORTED_BPC) { DRM_DEBUG_KMS("No DSC support for less than 8bpc\n");