| Message ID | 20241009054050.1796088-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dp_mst: Fix dsc mst bw overhead calculation | expand |
On Wed, 09 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > Fix the DSC flag assignment based on the dsc_slice_count returned > to avoid divide by zero error. > > Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC overhead during BW allocation") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 4765bda154c1..bacd294d6bb6 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, > if (dsc) { > int num_joined_pipes = intel_crtc_num_joined_pipes(crtc_state); > > - flags |= DRM_DP_BW_OVERHEAD_DSC; > dsc_slice_count = intel_dp_dsc_get_slice_count(connector, > adjusted_mode->clock, > adjusted_mode->hdisplay, > num_joined_pipes); > + /* > + * Try with dsc only if dsc_slice_count has a sane value i.e value is no > + * 0 > + */ > + if (dsc_slice_count) > + flags |= DRM_DP_BW_OVERHEAD_DSC; Do you think that's enough to handle the error?! BR, Jani. > } > > overhead = drm_dp_bw_overhead(crtc_state->lane_count,
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Wednesday, October 9, 2024 3:30 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Kandpal, Suraj > <suraj.kandpal@intel.com> > Subject: Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation > > On Wed, 09 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > > Fix the DSC flag assignment based on the dsc_slice_count returned to > > avoid divide by zero error. > > > > Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC > > overhead during BW allocation") > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 4765bda154c1..bacd294d6bb6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const > struct intel_crtc_state *crtc_state, > > if (dsc) { > > int num_joined_pipes = > intel_crtc_num_joined_pipes(crtc_state); > > > > - flags |= DRM_DP_BW_OVERHEAD_DSC; > > dsc_slice_count = intel_dp_dsc_get_slice_count(connector, > > adjusted_mode- > >clock, > > adjusted_mode- > >hdisplay, > > > num_joined_pipes); > > + /* > > + * Try with dsc only if dsc_slice_count has a sane value i.e > value is no > > + * 0 > > + */ > > + if (dsc_slice_count) > > + flags |= DRM_DP_BW_OVERHEAD_DSC; > > Do you think that's enough to handle the error?! Well this will make sure that if dsc_slice_count ends up being zero we don't take dsc overhead into account. Which should be enough to make sure we don't go and end up having a divide by zero error Regards, Suraj Kandpal > > BR, > Jani. > > > } > > > > overhead = drm_dp_bw_overhead(crtc_state->lane_count, > > -- > Jani Nikula, Intel
On Wed, Oct 09, 2024 at 11:10:50AM +0530, Suraj Kandpal wrote: > Fix the DSC flag assignment based on the dsc_slice_count returned > to avoid divide by zero error. > > Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC overhead during BW allocation") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 4765bda154c1..bacd294d6bb6 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, > if (dsc) { > int num_joined_pipes = intel_crtc_num_joined_pipes(crtc_state); > > - flags |= DRM_DP_BW_OVERHEAD_DSC; > dsc_slice_count = intel_dp_dsc_get_slice_count(connector, > adjusted_mode->clock, > adjusted_mode->hdisplay, > num_joined_pipes); > + /* > + * Try with dsc only if dsc_slice_count has a sane value i.e value is no > + * 0 > + */ > + if (dsc_slice_count) > + flags |= DRM_DP_BW_OVERHEAD_DSC; This would enable DSC, but with the wrong BW alloced. We'd need instead: https://lore.kernel.org/all/20241009110135.1216498-1-imre.deak@intel.com > } > > overhead = drm_dp_bw_overhead(crtc_state->lane_count, > -- > 2.43.2 >
On 10/9/2024 4:15 PM, Kandpal, Suraj wrote: > >> -----Original Message----- >> From: Jani Nikula <jani.nikula@linux.intel.com> >> Sent: Wednesday, October 9, 2024 3:30 PM >> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- >> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org >> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Kandpal, Suraj >> <suraj.kandpal@intel.com> >> Subject: Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation >> >> On Wed, 09 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: >>> Fix the DSC flag assignment based on the dsc_slice_count returned to >>> avoid divide by zero error. >>> >>> Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC >>> overhead during BW allocation") >>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> index 4765bda154c1..bacd294d6bb6 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const >> struct intel_crtc_state *crtc_state, >>> if (dsc) { >>> int num_joined_pipes = >> intel_crtc_num_joined_pipes(crtc_state); >>> - flags |= DRM_DP_BW_OVERHEAD_DSC; >>> dsc_slice_count = intel_dp_dsc_get_slice_count(connector, >>> adjusted_mode- >>> clock, >>> adjusted_mode- >>> hdisplay, >>> >> num_joined_pipes); >>> + /* >>> + * Try with dsc only if dsc_slice_count has a sane value i.e >> value is no >>> + * 0 >>> + */ >>> + if (dsc_slice_count) >>> + flags |= DRM_DP_BW_OVERHEAD_DSC; >> Do you think that's enough to handle the error?! > Well this will make sure that if dsc_slice_count ends up being zero we don't take dsc overhead into account. > Which should be enough to make sure we don't go and end up having a divide by zero error But the overhead computed will not be correct. I was thinking to avoid setting dsc flag if we dont get a valid slice_count. Perhaps in intel_dp_dsc_mst_compute_link_config() before calling intel_dp_mst_find_vcpi_slots_for_bpp() we should check for slice_count. Actually we again compute dsc slice_count quite late in dp_dsc_compute_config and set that in pipe_config->dsc.slice_count . Regards, Ankit > > Regards, > Suraj Kandpal >> BR, >> Jani. >> >>> } >>> >>> overhead = drm_dp_bw_overhead(crtc_state->lane_count, >> -- >> Jani Nikula, Intel
On 10/9/2024 4:37 PM, Imre Deak wrote: > On Wed, Oct 09, 2024 at 11:10:50AM +0530, Suraj Kandpal wrote: >> Fix the DSC flag assignment based on the dsc_slice_count returned >> to avoid divide by zero error. >> >> Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC overhead during BW allocation") >> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c >> index 4765bda154c1..bacd294d6bb6 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c >> @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, >> if (dsc) { >> int num_joined_pipes = intel_crtc_num_joined_pipes(crtc_state); >> >> - flags |= DRM_DP_BW_OVERHEAD_DSC; >> dsc_slice_count = intel_dp_dsc_get_slice_count(connector, >> adjusted_mode->clock, >> adjusted_mode->hdisplay, >> num_joined_pipes); >> + /* >> + * Try with dsc only if dsc_slice_count has a sane value i.e value is no >> + * 0 >> + */ >> + if (dsc_slice_count) >> + flags |= DRM_DP_BW_OVERHEAD_DSC; > This would enable DSC, but with the wrong BW alloced. We'd need instead: > https://lore.kernel.org/all/20241009110135.1216498-1-imre.deak@intel.com Right, I meant something like that, Imre beat me to it :) > >> } >> >> overhead = drm_dp_bw_overhead(crtc_state->lane_count, >> -- >> 2.43.2 >>
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 4765bda154c1..bacd294d6bb6 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -105,11 +105,16 @@ static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, if (dsc) { int num_joined_pipes = intel_crtc_num_joined_pipes(crtc_state); - flags |= DRM_DP_BW_OVERHEAD_DSC; dsc_slice_count = intel_dp_dsc_get_slice_count(connector, adjusted_mode->clock, adjusted_mode->hdisplay, num_joined_pipes); + /* + * Try with dsc only if dsc_slice_count has a sane value i.e value is no + * 0 + */ + if (dsc_slice_count) + flags |= DRM_DP_BW_OVERHEAD_DSC; } overhead = drm_dp_bw_overhead(crtc_state->lane_count,
Fix the DSC flag assignment based on the dsc_slice_count returned to avoid divide by zero error. Fixes: 4e0837a8d00a ("drm/i915/dp_mst: Account for FEC and DSC overhead during BW allocation") Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)