[PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
Fix intel_dp_dsc_compute_config, previously timeslots parameter
was used in fact not as a timeslots, but more like a ratio
timeslots/64, which of course didn't have any effect for SST DSC,
but causes now issues for MST DSC.
Secondly we need to calculate pipe_bpp using intel_dp_dsc_compute_bpp
only for SST DSC case, while for MST case it has been calculated
earlier already with intel_dp_dsc_mst_compute_link_config.
Third we also were wrongly determining sink min bpp/max bpp, those
limites should be intersected with our limits to find common
acceptable bpp's, plus on top of that we should align those with
VESA bpps and only then calculate required timeslots amount.
Some MST hubs started to work only after third change was made.
v2: Make kernel test robot happy(claimed there was unitialzed use,
while there is none)
v3: Rename intel_dp_dsc_nearest_vesa_bpp to intel_dp_dsc_nearest_valid_bpp
(Manasi Navare)
Reviewed-by: Manasi Navare
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_dp.c | 69 ++---
drivers/gpu/drm/i915/display/intel_dp.h | 3 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 69 +
3 files changed, 106 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index d78216fba0a2..533ee1d01366 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -717,9 +717,14 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private
*i915,
* for SST -> TimeSlotsPerMTP is 1,
* for MST -> TimeSlotsPerMTP has to be calculated
*/
- bits_per_pixel = (link_clock * lane_count * 8) * timeslots /
-intel_dp_mode_to_fec_clock(mode_clock);
- drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel);
+ bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots,
+ intel_dp_mode_to_fec_clock(mode_clock) *
8);
+
+ drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots "
+ "total bw %u pixel clock %u\n",
+ bits_per_pixel, timeslots,
+ (link_clock * lane_count * 8),
+ intel_dp_mode_to_fec_clock(mode_clock));
/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
@@ -1048,7 +1053,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
target_clock,
mode->hdisplay,
bigjoiner,
- pipe_bpp, 1) >> 4;
+ pipe_bpp, 64) >> 4;
dsc_slice_count =
intel_dp_dsc_get_slice_count(intel_dp,
target_clock,
@@ -1482,7 +1487,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state,
struct link_config_limits *limits,
- int timeslots)
+ int timeslots,
+ bool compute_pipe_bpp)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
@@ -1497,7 +1503,10 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
if (!intel_dp_supports_dsc(intel_dp, pipe_config))
return -EINVAL;
- pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ if (compute_pipe_bpp)
+ pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ else
+ pipe_bpp = pipe_config->pipe_bpp;
if (intel_dp->force_dsc_bpc) {
pipe_bpp = intel_dp->force_dsc_bpc * 3;
@@ -1528,31 +1537,47 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
true);
} else {
- u16 dsc_max_output_bpp;
+ u16 dsc_max_output_bpp = 0;
u8 dsc_dp_slice_count;
- dsc_max_output_bpp =
- intel_dp_dsc_get_output_bpp(dev_priv,
- pipe_config->port_clock,
- pipe_config->lane_count,
- adjusted_mode->crtc_clock,
-
RE: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
Thanks Stan for the explanation, With that Reviewed-by: Manasi Navare Manasi -Original Message- From: Lisovskiy, Stanislav Sent: Tuesday, November 22, 2022 2:40 AM To: Navare, Manasi D Cc: intel-...@lists.freedesktop.org; Saarinen, Jani ; Nikula, Jani ; dri-devel@lists.freedesktop.org; Govindapillai, Vinod Subject: Re: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC On Thu, Nov 10, 2022 at 02:23:53PM -0800, Navare, Manasi wrote: > On Thu, Nov 03, 2022 at 03:23:00PM +0200, Stanislav Lisovskiy wrote: > > Fix intel_dp_dsc_compute_config, previously timeslots parameter was > > used in fact not as a timeslots, but more like a ratio timeslots/64, > > which of course didn't have any effect for SST DSC, but causes now > > issues for MST DSC. > > Secondly we need to calculate pipe_bpp using > > intel_dp_dsc_compute_bpp only for SST DSC case, while for MST case > > it has been calculated earlier already with > > intel_dp_dsc_mst_compute_link_config. > > Third we also were wrongly determining sink min bpp/max bpp, those > > limites should be intersected with our limits to find common > > acceptable bpp's, plus on top of that we should align those with > > VESA bpps and only then calculate required timeslots amount. > > Some MST hubs started to work only after third change was made. > > > > v2: Make kernel test robot happy(claimed there was unitialzed use, > > while there is none) > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 69 ++--- > > drivers/gpu/drm/i915/display/intel_dp.h | 3 +- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 69 > > + > > 3 files changed, 106 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 8288a30dbd51..82752b696498 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -716,9 +716,14 @@ u16 intel_dp_dsc_get_output_bpp(struct > > drm_i915_private *i915, > > * for SST -> TimeSlotsPerMTP is 1, > > * for MST -> TimeSlotsPerMTP has to be calculated > > */ > > - bits_per_pixel = (link_clock * lane_count * 8) * timeslots / > > -intel_dp_mode_to_fec_clock(mode_clock); > > - drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel); > > + bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots, > > + intel_dp_mode_to_fec_clock(mode_clock) * > > 8); > > Why did we remove the *8 in the numerator for the total bandwidth > link_clock * lane_count * 8 ? > > Other than this clarification, all changes look good > > Manasi Hi Manasi, Because previously this function was actually confusing the ratio timeslots/64, with the timeslots number. It was actually expecting a ratio timeslots/64, rather than the timeslots number. For SST it didn't matter as timeslots were always 1, but for MST case if we multiply that by number number of timeslots, this formula will return some big bogus bits_per_pixel number(checked that). Of course we can pass a ratio timeslots/64 here, but it isn't very convenient and intuitive to manipulate. So I made it to use a "timeslots" parameter as timeslots number, so that the ratio is calculated as part of the formula i.e: ((link_clock * lane_count * 8) * (timeslots / 64)) / intel_dp_mode_to_fec_clock(mode_clock); which can be simplified as ((link_clock * lane_count * timeslots) / 8) / intel_dp_mode_to_fec_clock(mode_clock); the whole formula comes from that pipe_bpp * crtc_clock should be equal to link_total_bw * (timeslots / 64), i.e timeslots/64 ratio defines, how much of the link_total_bw(link_clock * lane_count * 8) we have for those pipe_bpp * crtc_clock, which we want to accomodate there. Obviously if we just multiplied link_total_bw by timeslots, we would get a situation that the more timeslots we allocate, the more total bw we get, which is wrong and will result in some bogus huge pipe_bpp numbers. Stan > > > + > > + drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots " > > + "total bw %u pixel clock %u\n", > > + bits_per_pixel, timeslots, > > + (link_clock * lane_count * 8), > > + intel_dp_mode_to_fec_clock(mode_clock)); > > > > /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */ > > max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915)
Re: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
On Thu, Nov 10, 2022 at 02:23:53PM -0800, Navare, Manasi wrote: > On Thu, Nov 03, 2022 at 03:23:00PM +0200, Stanislav Lisovskiy wrote: > > Fix intel_dp_dsc_compute_config, previously timeslots parameter > > was used in fact not as a timeslots, but more like a ratio > > timeslots/64, which of course didn't have any effect for SST DSC, > > but causes now issues for MST DSC. > > Secondly we need to calculate pipe_bpp using intel_dp_dsc_compute_bpp > > only for SST DSC case, while for MST case it has been calculated > > earlier already with intel_dp_dsc_mst_compute_link_config. > > Third we also were wrongly determining sink min bpp/max bpp, those > > limites should be intersected with our limits to find common > > acceptable bpp's, plus on top of that we should align those with > > VESA bpps and only then calculate required timeslots amount. > > Some MST hubs started to work only after third change was made. > > > > v2: Make kernel test robot happy(claimed there was unitialzed use, > > while there is none) > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 69 ++--- > > drivers/gpu/drm/i915/display/intel_dp.h | 3 +- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 69 + > > 3 files changed, 106 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 8288a30dbd51..82752b696498 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -716,9 +716,14 @@ u16 intel_dp_dsc_get_output_bpp(struct > > drm_i915_private *i915, > > * for SST -> TimeSlotsPerMTP is 1, > > * for MST -> TimeSlotsPerMTP has to be calculated > > */ > > - bits_per_pixel = (link_clock * lane_count * 8) * timeslots / > > -intel_dp_mode_to_fec_clock(mode_clock); > > - drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel); > > + bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots, > > + intel_dp_mode_to_fec_clock(mode_clock) * > > 8); > > Why did we remove the *8 in the numerator for the total bandwidth > link_clock * lane_count * 8 ? > > Other than this clarification, all changes look good > > Manasi Hi Manasi, Because previously this function was actually confusing the ratio timeslots/64, with the timeslots number. It was actually expecting a ratio timeslots/64, rather than the timeslots number. For SST it didn't matter as timeslots were always 1, but for MST case if we multiply that by number number of timeslots, this formula will return some big bogus bits_per_pixel number(checked that). Of course we can pass a ratio timeslots/64 here, but it isn't very convenient and intuitive to manipulate. So I made it to use a "timeslots" parameter as timeslots number, so that the ratio is calculated as part of the formula i.e: ((link_clock * lane_count * 8) * (timeslots / 64)) / intel_dp_mode_to_fec_clock(mode_clock); which can be simplified as ((link_clock * lane_count * timeslots) / 8) / intel_dp_mode_to_fec_clock(mode_clock); the whole formula comes from that pipe_bpp * crtc_clock should be equal to link_total_bw * (timeslots / 64), i.e timeslots/64 ratio defines, how much of the link_total_bw(link_clock * lane_count * 8) we have for those pipe_bpp * crtc_clock, which we want to accomodate there. Obviously if we just multiplied link_total_bw by timeslots, we would get a situation that the more timeslots we allocate, the more total bw we get, which is wrong and will result in some bogus huge pipe_bpp numbers. Stan > > > + > > + drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots " > > + "total bw %u pixel clock %u\n", > > + bits_per_pixel, timeslots, > > + (link_clock * lane_count * 8), > > + intel_dp_mode_to_fec_clock(mode_clock)); > > > > /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */ > > max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) / > > @@ -1047,7 +1052,7 @@ intel_dp_mode_valid(struct drm_connector *_connector, > > target_clock, > > mode->hdisplay, > > bigjoiner, > > - pipe_bpp, 1) >> 4; > > + pipe_bpp, 64) >> 4; > > dsc_slice_count = > > intel_dp_dsc_get_slice_count(intel_dp, > > target_clock, > > @@ -1481,7 +1486,8 @@ int intel_dp_dsc_compute_config(struct intel_dp > > *intel_dp, > > struct intel_crtc_state *pipe_config, >
Re: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
On Thu, Nov 03, 2022 at 03:23:00PM +0200, Stanislav Lisovskiy wrote:
> Fix intel_dp_dsc_compute_config, previously timeslots parameter
> was used in fact not as a timeslots, but more like a ratio
> timeslots/64, which of course didn't have any effect for SST DSC,
> but causes now issues for MST DSC.
> Secondly we need to calculate pipe_bpp using intel_dp_dsc_compute_bpp
> only for SST DSC case, while for MST case it has been calculated
> earlier already with intel_dp_dsc_mst_compute_link_config.
> Third we also were wrongly determining sink min bpp/max bpp, those
> limites should be intersected with our limits to find common
> acceptable bpp's, plus on top of that we should align those with
> VESA bpps and only then calculate required timeslots amount.
> Some MST hubs started to work only after third change was made.
>
> v2: Make kernel test robot happy(claimed there was unitialzed use,
> while there is none)
>
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 69 ++---
> drivers/gpu/drm/i915/display/intel_dp.h | 3 +-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 69 +
> 3 files changed, 106 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8288a30dbd51..82752b696498 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -716,9 +716,14 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private
> *i915,
>* for SST -> TimeSlotsPerMTP is 1,
>* for MST -> TimeSlotsPerMTP has to be calculated
>*/
> - bits_per_pixel = (link_clock * lane_count * 8) * timeslots /
> - intel_dp_mode_to_fec_clock(mode_clock);
> - drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel);
> + bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots,
> + intel_dp_mode_to_fec_clock(mode_clock) *
> 8);
Why did we remove the *8 in the numerator for the total bandwidth
link_clock * lane_count * 8 ?
Other than this clarification, all changes look good
Manasi
> +
> + drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots "
> + "total bw %u pixel clock %u\n",
> + bits_per_pixel, timeslots,
> + (link_clock * lane_count * 8),
> + intel_dp_mode_to_fec_clock(mode_clock));
>
> /* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
> max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
> @@ -1047,7 +1052,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> target_clock,
> mode->hdisplay,
> bigjoiner,
> - pipe_bpp, 1) >> 4;
> + pipe_bpp, 64) >> 4;
> dsc_slice_count =
> intel_dp_dsc_get_slice_count(intel_dp,
>target_clock,
> @@ -1481,7 +1486,8 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
> struct intel_crtc_state *pipe_config,
> struct drm_connector_state *conn_state,
> struct link_config_limits *limits,
> - int timeslots)
> + int timeslots,
> + bool compute_pipe_bpp)
> {
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> @@ -1496,7 +1502,10 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
> if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> return -EINVAL;
>
> - pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
> conn_state->max_requested_bpc);
> + if (compute_pipe_bpp)
> + pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
> conn_state->max_requested_bpc);
> + else
> + pipe_bpp = pipe_config->pipe_bpp;
>
> if (intel_dp->force_dsc_bpc) {
> pipe_bpp = intel_dp->force_dsc_bpc * 3;
> @@ -1527,31 +1536,47 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
> drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> true);
> } else {
> - u16 dsc_max_output_bpp;
> + u16 dsc_max_output_bpp = 0;
> u8 dsc_dp_slice_count;
>
> - dsc_max_output_bpp =
> - intel_dp_dsc_get_output_bpp(dev_priv,
> -
[PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
Fix intel_dp_dsc_compute_config, previously timeslots parameter
was used in fact not as a timeslots, but more like a ratio
timeslots/64, which of course didn't have any effect for SST DSC,
but causes now issues for MST DSC.
Secondly we need to calculate pipe_bpp using intel_dp_dsc_compute_bpp
only for SST DSC case, while for MST case it has been calculated
earlier already with intel_dp_dsc_mst_compute_link_config.
Third we also were wrongly determining sink min bpp/max bpp, those
limites should be intersected with our limits to find common
acceptable bpp's, plus on top of that we should align those with
VESA bpps and only then calculate required timeslots amount.
Some MST hubs started to work only after third change was made.
v2: Make kernel test robot happy(claimed there was unitialzed use,
while there is none)
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_dp.c | 69 ++---
drivers/gpu/drm/i915/display/intel_dp.h | 3 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 69 +
3 files changed, 106 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 8288a30dbd51..82752b696498 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -716,9 +716,14 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private
*i915,
* for SST -> TimeSlotsPerMTP is 1,
* for MST -> TimeSlotsPerMTP has to be calculated
*/
- bits_per_pixel = (link_clock * lane_count * 8) * timeslots /
-intel_dp_mode_to_fec_clock(mode_clock);
- drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel);
+ bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots,
+ intel_dp_mode_to_fec_clock(mode_clock) *
8);
+
+ drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots "
+ "total bw %u pixel clock %u\n",
+ bits_per_pixel, timeslots,
+ (link_clock * lane_count * 8),
+ intel_dp_mode_to_fec_clock(mode_clock));
/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
@@ -1047,7 +1052,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
target_clock,
mode->hdisplay,
bigjoiner,
- pipe_bpp, 1) >> 4;
+ pipe_bpp, 64) >> 4;
dsc_slice_count =
intel_dp_dsc_get_slice_count(intel_dp,
target_clock,
@@ -1481,7 +1486,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state,
struct link_config_limits *limits,
- int timeslots)
+ int timeslots,
+ bool compute_pipe_bpp)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
@@ -1496,7 +1502,10 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
if (!intel_dp_supports_dsc(intel_dp, pipe_config))
return -EINVAL;
- pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ if (compute_pipe_bpp)
+ pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ else
+ pipe_bpp = pipe_config->pipe_bpp;
if (intel_dp->force_dsc_bpc) {
pipe_bpp = intel_dp->force_dsc_bpc * 3;
@@ -1527,31 +1536,47 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
true);
} else {
- u16 dsc_max_output_bpp;
+ u16 dsc_max_output_bpp = 0;
u8 dsc_dp_slice_count;
- dsc_max_output_bpp =
- intel_dp_dsc_get_output_bpp(dev_priv,
- pipe_config->port_clock,
- pipe_config->lane_count,
- adjusted_mode->crtc_clock,
-
adjusted_mode->crtc_hdisplay,
-
Re: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
Hi Stanislav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on drm/drm-next linus/master v6.1-rc3 next-20221101]
[cannot apply to drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Stanislav-Lisovskiy/Add-DP-MST-DSC-support-to-i915/20221101-174550
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:
https://lore.kernel.org/r/20221101094222.22091-7-stanislav.lisovskiy%40intel.com
patch subject: [PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST
DSC
config: i386-randconfig-a004
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/effdb0df885df736ad89da1a602dc43f82598408
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Stanislav-Lisovskiy/Add-DP-MST-DSC-support-to-i915/20221101-174550
git checkout effdb0df885df736ad89da1a602dc43f82598408
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_dp.c:1543:7: warning: variable
>> 'dsc_max_output_bpp' is used uninitialized whenever 'if' condition is false
>> [-Wsometimes-uninitialized]
if (compute_pipe_bpp) {
^~~~
drivers/gpu/drm/i915/display/intel_dp.c:1559:9: note: uninitialized use
occurs here
if ((!dsc_max_output_bpp && compute_pipe_bpp) ||
!dsc_dp_slice_count) {
^~
drivers/gpu/drm/i915/display/intel_dp.c:1543:3: note: remove the 'if' if its
condition is always true
if (compute_pipe_bpp) {
^~
drivers/gpu/drm/i915/display/intel_dp.c:1540:25: note: initialize the
variable 'dsc_max_output_bpp' to silence this warning
u16 dsc_max_output_bpp;
^
= 0
1 warning generated.
vim +1543 drivers/gpu/drm/i915/display/intel_dp.c
1485
1486 int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
1487 struct intel_crtc_state *pipe_config,
1488 struct drm_connector_state *conn_state,
1489 struct link_config_limits *limits,
1490 int timeslots,
1491 bool compute_pipe_bpp)
1492 {
1493 struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
1494 struct drm_i915_private *dev_priv =
to_i915(dig_port->base.base.dev);
1495 const struct drm_display_mode *adjusted_mode =
1496 _config->hw.adjusted_mode;
1497 int pipe_bpp;
1498 int ret;
1499
1500 pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
1501 intel_dp_supports_fec(intel_dp, pipe_config);
1502
1503 if (!intel_dp_supports_dsc(intel_dp, pipe_config))
1504 return -EINVAL;
1505
1506 if (compute_pipe_bpp)
1507 pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
1508 else
1509 pipe_bpp = pipe_config->pipe_bpp;
1510
1511 if (intel_dp->force_dsc_bpc) {
1512 pipe_bpp = intel_dp->force_dsc_bpc * 3;
1513 drm_dbg_kms(_priv->drm, "Input DSC BPP forced to
%d", pipe_bpp);
1514 }
1515
1516 /* Min Input BPC for ICL+ is 8 */
1517 if (pipe_bpp < 8 * 3) {
1518 drm_dbg_kms(_priv->drm,
1519 "No DSC support for less than 8bpc\n");
1520 return -EINVAL;
1521 }
1522
1523 /*
1524 * For now enable DSC for max bpp, max link rate, max lane
count.
1525 * Optimize this later for the minimum possible link rate/lane
count
1526 * with DSC enabled for the requested mode.
1527 */
[PATCH 6/6] drm/i915: Bpp/timeslot calculation fixes for DP MST DSC
Fix intel_dp_dsc_compute_config, previously timeslots parameter
was used in fact not as a timeslots, but more like a ratio
timeslots/64, which of course didn't have any effect for SST DSC,
but causes now issues for MST DSC.
Secondly we need to calculate pipe_bpp using intel_dp_dsc_compute_bpp
only for SST DSC case, while for MST case it has been calculated
earlier already with intel_dp_dsc_mst_compute_link_config.
Third we also were wrongly determining sink min bpp/max bpp, those
limites should be intersected with our limits to find common
acceptable bpp's, plus on top of that we should align those with
VESA bpps and only then calculate required timeslots amount.
Some MST hubs started to work only after third change was made.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/display/intel_dp.c | 52 ++---
drivers/gpu/drm/i915/display/intel_dp.h | 3 +-
drivers/gpu/drm/i915/display/intel_dp_mst.c | 65 +
3 files changed, 88 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 71e08e665065..706fa4f7ea62 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -717,9 +717,14 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private
*i915,
* for SST -> TimeSlotsPerMTP is 1,
* for MST -> TimeSlotsPerMTP has to be calculated
*/
- bits_per_pixel = (link_clock * lane_count * 8) * timeslots /
-intel_dp_mode_to_fec_clock(mode_clock);
- drm_dbg_kms(>drm, "Max link bpp: %u\n", bits_per_pixel);
+ bits_per_pixel = DIV_ROUND_UP((link_clock * lane_count) * timeslots,
+ intel_dp_mode_to_fec_clock(mode_clock) *
8);
+
+ drm_dbg_kms(>drm, "Max link bpp is %u for %u timeslots "
+ "total bw %u pixel clock %u\n",
+ bits_per_pixel, timeslots,
+ (link_clock * lane_count * 8),
+ intel_dp_mode_to_fec_clock(mode_clock));
/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
@@ -1048,7 +1053,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
target_clock,
mode->hdisplay,
bigjoiner,
- pipe_bpp, 1) >> 4;
+ pipe_bpp, 64) >> 4;
dsc_slice_count =
intel_dp_dsc_get_slice_count(intel_dp,
target_clock,
@@ -1482,7 +1487,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state,
struct link_config_limits *limits,
- int timeslots)
+ int timeslots,
+ bool compute_pipe_bpp)
{
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
@@ -1497,7 +1503,10 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
if (!intel_dp_supports_dsc(intel_dp, pipe_config))
return -EINVAL;
- pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ if (compute_pipe_bpp)
+ pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp,
conn_state->max_requested_bpc);
+ else
+ pipe_bpp = pipe_config->pipe_bpp;
if (intel_dp->force_dsc_bpc) {
pipe_bpp = intel_dp->force_dsc_bpc * 3;
@@ -1531,28 +1540,31 @@ int intel_dp_dsc_compute_config(struct intel_dp
*intel_dp,
u16 dsc_max_output_bpp;
u8 dsc_dp_slice_count;
- dsc_max_output_bpp =
- intel_dp_dsc_get_output_bpp(dev_priv,
- pipe_config->port_clock,
- pipe_config->lane_count,
- adjusted_mode->crtc_clock,
-
adjusted_mode->crtc_hdisplay,
-
pipe_config->bigjoiner_pipes,
- pipe_bpp,
- timeslots);
+ if (compute_pipe_bpp) {
+ dsc_max_output_bpp =
+
