[PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Suraj Kandpal]

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;
 	}
 
 	overhead = drm_dp_bw_overhead(crtc_state->lane_count,
-- 
2.43.2

✓ Fi.CI.BAT: success for drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4479 bytes --]

== Series Details ==

Series: drm/i915/dp_mst: Fix dsc mst bw overhead calculation
URL   : https://patchwork.freedesktop.org/series/139743/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_15496 -> Patchwork_139743v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-arls-1 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_139743v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@load:
    - bat-adlp-6:         [PASS][1] -> [DMESG-WARN][2] ([i915#12253])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-adlp-6/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-adlp-6/igt@i915_module_load@load.html

  * igt@i915_selftest@live:
    - bat-arlh-2:         [PASS][3] -> [ABORT][4] ([i915#12133])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arlh-2/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-arlh-2/igt@i915_selftest@live.html
    - bat-arls-2:         [PASS][5] -> [ABORT][6] ([i915#12061] / [i915#12133])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arls-2/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-arls-2/igt@i915_selftest@live.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [PASS][7] -> [DMESG-WARN][8] ([i915#11621]) +132 other tests dmesg-warn
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-2:         [PASS][9] -> [ABORT][10] ([i915#12061])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arlh-2/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-arlh-2/igt@i915_selftest@live@workarounds.html
    - bat-arls-2:         [PASS][11] -> [ABORT][12] ([i915#12061])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-arls-2/igt@i915_selftest@live@workarounds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-arls-2/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [ABORT][13] ([i915#12216]) -> [PASS][14] +1 other test pass
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-mtlp-8/igt@i915_selftest@live.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-mtlp-8/igt@i915_selftest@live.html
    - bat-twl-1:          [INCOMPLETE][15] ([i915#12133]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-twl-1/igt@i915_selftest@live.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-twl-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@gt_lrc:
    - bat-twl-1:          [INCOMPLETE][17] ([i915#10886]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15496/bat-twl-1/igt@i915_selftest@live@gt_lrc.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/bat-twl-1/igt@i915_selftest@live@gt_lrc.html

  
  [i915#10886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10886
  [i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12133]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12133
  [i915#12216]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12216
  [i915#12253]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12253


Build changes
-------------

  * Linux: CI_DRM_15496 -> Patchwork_139743v1

  CI-20190529: 20190529
  CI_DRM_15496: 2c8a08f54befdd831076e0854fe311569c209835 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8061: 8061
  Patchwork_139743v1: 2c8a08f54befdd831076e0854fe311569c209835 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139743v1/index.html

[-- Attachment #2: Type: text/html, Size: 5521 bytes --]

Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Jani Nikula]

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,

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Kandpal, Suraj]

> -----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

Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Nautiyal, Ankit K]

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

Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Imre Deak]

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
> 

Re: [PATCH] drm/i915/dp_mst: Fix dsc mst bw overhead calculation

[Nautiyal, Ankit K]

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
>>