Re: [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-06-15 Thread Dave Airlie
On Wed, 3 May 2023 at 22:23, Lisovskiy, Stanislav
 wrote:
>
> On Wed, May 03, 2023 at 02:07:04PM +0300, Ville Syrjälä wrote:
> > On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> > > On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä 
> > > >
> > > > The MST DSC code has a myriad of issues:
> > > > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > > > - Return values of .mode_valid_ctx() are wrong
> > > > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> > > >   of the code doesn't agree
> > > > - compressed bpp calculations don't make sense
> > > > - FEC handling needs to consider the entire link as opposed to just
> > > >   the single stream. Currently FEC would only get enabled if the
> > > >   first enabled stream is compressed. Also I'm not seeing anything
> > > >   that would account for the FEC overhead in any bandwidth calculations
> > > > - PPS SDP is only handled for the first stream via the dig_port
> > > >   hooks, other streams will not be transmittitng any PPS SDPs
> > > > - PPS SDP readout is missing (also missing for SST!)
> > > > - VDSC readout is missing (also missing for SST!)
> > > >
> > > > The FEC issues is really the big one since we have no way currently
> > > > to apply such link wide configuration constraints. Changing that is
> > > > going to require a much bigger rework of the higher level modeset
> > > > .compute_config() logic. We will also need such a rework to properly
> > > > distribute the available bandwidth across all the streams on the
> > > > same link (which is a must to eg. enable deep color).
> > >
> > > Also all the things you mentioned are subject for discussion, for example
> > > I see that FEC overhead is actually accounted for bpp calculation for 
> > > instance.
> >
> > AFAICS FEC is only accounted for in the data M/N calculations,
> > assuming that particular stream happened to be compressed. I'm
> > not sure if that actually matters since at least the link M/N
> > are not even used by the MST sink. I suppose the data M/N might
> > still be used for something though. For any uncompressed stream
> > on the same link the data M/N values will be calculated
> > incorrectly without FEC.
> >
> > And as mentioned, the FEC bandwidth overhead doesn't seem to
> > be accounted anywhere so no guarantee that we won't try to
> > oversubcribe the link.
> >
> > And FEC will only be enabled if the first stream to be enabled
> > is compressed, otherwise we will enable the link without FEC
> > and still try to cram other compressed streams through it
> > (albeit without the PPS SDP so who knows what will happen)
> > and that is illegal.
> >
> > > We usually improve things by gradually fixing, because if we act same way 
> > > towards
> > > all wrong code in the driver, we could end up removing the whole i915.
> >
> > We ususally don't merge code that has this many obvious and/or
> > fundemental issues.
>
> Well, this is arguable and subjective judgement. Fact is that, so far we had 
> more MST hubs
> working with that code than without. Also no regressions or anything like 
> that.
> Moreover we usually merge code after code review, in particular those patches
> did spend lots of time in review, where you could comment also.
>
> Regarding merging code with fundamental issues, just recently you had 
> admitted yourself
> that bigjoiner issue for instance, we had recently, was partly caused by your 
> code, because
> we don't anymore copy the pll state to slave crtc.
> I would say that words like "obvious" and "fundamental"
> issues can be applied to many things, however I thought that we always fix 
> things in constructive,
> but not destructive/negative way.
> Should I call also all code completely broken and remove it, once we discover 
> some flaws
> there? Oh, we had many regressions, where I could say the same.
>
> And once again I'm completely okay, if you did introduce better functionality 
> instead
> AND I know you have some valid points there, but now we are just removing 
> everything completely,
> without providing anything better.
>
> But okay, I've mentioned what I think about this and from side this is nak.
> And once the guys to whom those patches helped will pop up from gitlab,
> asking why their MST hubs stopped working - I will just refer them here.
>
> >
> > Now, most of the issues I listed above are probably fixable
> > in a way that could be backported to stable kernels, but
> > unfortunately the FEC issue is not one of those. That one
> > will likely need massive amounts of work all over the driver
> > modeset code, making a backport impossible.
> >
> > > So from my side I would nack it, at least until you have a code which 
> > > handles
> > > all of this better - I have no doubt you probably have some ideas in your 
> > > mind, so lets be constructive at least and propose something better first.
> > > This code doesn't cause any regressions, but still provides 

Re: [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-05-03 Thread Lisovskiy, Stanislav
On Wed, May 03, 2023 at 02:07:04PM +0300, Ville Syrjälä wrote:
> On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> > On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > The MST DSC code has a myriad of issues:
> > > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > > - Return values of .mode_valid_ctx() are wrong
> > > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> > >   of the code doesn't agree
> > > - compressed bpp calculations don't make sense
> > > - FEC handling needs to consider the entire link as opposed to just
> > >   the single stream. Currently FEC would only get enabled if the
> > >   first enabled stream is compressed. Also I'm not seeing anything
> > >   that would account for the FEC overhead in any bandwidth calculations
> > > - PPS SDP is only handled for the first stream via the dig_port
> > >   hooks, other streams will not be transmittitng any PPS SDPs
> > > - PPS SDP readout is missing (also missing for SST!)
> > > - VDSC readout is missing (also missing for SST!)
> > > 
> > > The FEC issues is really the big one since we have no way currently
> > > to apply such link wide configuration constraints. Changing that is
> > > going to require a much bigger rework of the higher level modeset
> > > .compute_config() logic. We will also need such a rework to properly
> > > distribute the available bandwidth across all the streams on the
> > > same link (which is a must to eg. enable deep color).
> > 
> > Also all the things you mentioned are subject for discussion, for example
> > I see that FEC overhead is actually accounted for bpp calculation for 
> > instance.
> 
> AFAICS FEC is only accounted for in the data M/N calculations,
> assuming that particular stream happened to be compressed. I'm
> not sure if that actually matters since at least the link M/N
> are not even used by the MST sink. I suppose the data M/N might
> still be used for something though. For any uncompressed stream
> on the same link the data M/N values will be calculated
> incorrectly without FEC.
> 
> And as mentioned, the FEC bandwidth overhead doesn't seem to
> be accounted anywhere so no guarantee that we won't try to
> oversubcribe the link.
> 
> And FEC will only be enabled if the first stream to be enabled
> is compressed, otherwise we will enable the link without FEC
> and still try to cram other compressed streams through it
> (albeit without the PPS SDP so who knows what will happen)
> and that is illegal.
> 
> > We usually improve things by gradually fixing, because if we act same way 
> > towards
> > all wrong code in the driver, we could end up removing the whole i915.
> 
> We ususally don't merge code that has this many obvious and/or
> fundemental issues.

Well, this is arguable and subjective judgement. Fact is that, so far we had 
more MST hubs
working with that code than without. Also no regressions or anything like that.
Moreover we usually merge code after code review, in particular those patches
did spend lots of time in review, where you could comment also.

Regarding merging code with fundamental issues, just recently you had admitted 
yourself
that bigjoiner issue for instance, we had recently, was partly caused by your 
code, because
we don't anymore copy the pll state to slave crtc. 
I would say that words like "obvious" and "fundamental"
issues can be applied to many things, however I thought that we always fix 
things in constructive,
but not destructive/negative way. 
Should I call also all code completely broken and remove it, once we discover 
some flaws 
there? Oh, we had many regressions, where I could say the same.

And once again I'm completely okay, if you did introduce better functionality 
instead
AND I know you have some valid points there, but now we are just removing 
everything completely,
without providing anything better.

But okay, I've mentioned what I think about this and from side this is nak. 
And once the guys to whom those patches helped will pop up from gitlab,
asking why their MST hubs stopped working - I will just refer them here.

> 
> Now, most of the issues I listed above are probably fixable
> in a way that could be backported to stable kernels, but
> unfortunately the FEC issue is not one of those. That one
> will likely need massive amounts of work all over the driver
> modeset code, making a backport impossible.
> 
> > So from my side I would nack it, at least until you have a code which 
> > handles
> > all of this better - I have no doubt you probably have some ideas in your 
> > mind, so lets be constructive at least and propose something better first.
> > This code doesn't cause any regressions, but still provides "some" support 
> > to DP MST DSC to say the least and even if that would be removed, if some 
> > of those users 
> > refer to me, I would probably then just point to this mail discussion 
> > everytime.
> 
> It seems very likely that it 

Re: [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-05-03 Thread Ville Syrjälä
On Wed, May 03, 2023 at 10:36:42AM +0300, Lisovskiy, Stanislav wrote:
> On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > The MST DSC code has a myriad of issues:
> > - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> > - Return values of .mode_valid_ctx() are wrong
> > - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
> >   of the code doesn't agree
> > - compressed bpp calculations don't make sense
> > - FEC handling needs to consider the entire link as opposed to just
> >   the single stream. Currently FEC would only get enabled if the
> >   first enabled stream is compressed. Also I'm not seeing anything
> >   that would account for the FEC overhead in any bandwidth calculations
> > - PPS SDP is only handled for the first stream via the dig_port
> >   hooks, other streams will not be transmittitng any PPS SDPs
> > - PPS SDP readout is missing (also missing for SST!)
> > - VDSC readout is missing (also missing for SST!)
> > 
> > The FEC issues is really the big one since we have no way currently
> > to apply such link wide configuration constraints. Changing that is
> > going to require a much bigger rework of the higher level modeset
> > .compute_config() logic. We will also need such a rework to properly
> > distribute the available bandwidth across all the streams on the
> > same link (which is a must to eg. enable deep color).
> 
> Also all the things you mentioned are subject for discussion, for example
> I see that FEC overhead is actually accounted for bpp calculation for 
> instance.

AFAICS FEC is only accounted for in the data M/N calculations,
assuming that particular stream happened to be compressed. I'm
not sure if that actually matters since at least the link M/N
are not even used by the MST sink. I suppose the data M/N might
still be used for something though. For any uncompressed stream
on the same link the data M/N values will be calculated
incorrectly without FEC.

And as mentioned, the FEC bandwidth overhead doesn't seem to
be accounted anywhere so no guarantee that we won't try to
oversubcribe the link.

And FEC will only be enabled if the first stream to be enabled
is compressed, otherwise we will enable the link without FEC
and still try to cram other compressed streams through it
(albeit without the PPS SDP so who knows what will happen)
and that is illegal.

> We usually improve things by gradually fixing, because if we act same way 
> towards
> all wrong code in the driver, we could end up removing the whole i915.

We ususally don't merge code that has this many obvious and/or
fundemental issues.

Now, most of the issues I listed above are probably fixable
in a way that could be backported to stable kernels, but
unfortunately the FEC issue is not one of those. That one
will likely need massive amounts of work all over the driver
modeset code, making a backport impossible.

> So from my side I would nack it, at least until you have a code which handles
> all of this better - I have no doubt you probably have some ideas in your 
> mind, so lets be constructive at least and propose something better first.
> This code doesn't cause any regressions, but still provides "some" support to 
> DP MST DSC to say the least and even if that would be removed, if some of 
> those users 
> refer to me, I would probably then just point to this mail discussion 
> everytime.

It seems very likely that it will cause regressions at some point,
it just needs a specific multi-display MST setup. The resulting
problems will be very confusing to debug since the order in which
you enable/disable the outputs will have an impact on what actually
goes wrong on account of the FEC and PPS SDP issues. The longer
we wait disabling this the harder it will be to deal with those
regressions since we the probably can't revert anymore (a straight
revert was already not possible) but also can't fix it in a way
that can be backported (due to the FEC issues in particular).

-- 
Ville Syrjälä
Intel


Re: [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-05-03 Thread Lisovskiy, Stanislav
On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> The MST DSC code has a myriad of issues:
> - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> - Return values of .mode_valid_ctx() are wrong
> - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
>   of the code doesn't agree
> - compressed bpp calculations don't make sense
> - FEC handling needs to consider the entire link as opposed to just
>   the single stream. Currently FEC would only get enabled if the
>   first enabled stream is compressed. Also I'm not seeing anything
>   that would account for the FEC overhead in any bandwidth calculations
> - PPS SDP is only handled for the first stream via the dig_port
>   hooks, other streams will not be transmittitng any PPS SDPs
> - PPS SDP readout is missing (also missing for SST!)
> - VDSC readout is missing (also missing for SST!)
> 
> The FEC issues is really the big one since we have no way currently
> to apply such link wide configuration constraints. Changing that is
> going to require a much bigger rework of the higher level modeset
> .compute_config() logic. We will also need such a rework to properly
> distribute the available bandwidth across all the streams on the
> same link (which is a must to eg. enable deep color).

Also all the things you mentioned are subject for discussion, for example
I see that FEC overhead is actually accounted for bpp calculation for instance.
We usually improve things by gradually fixing, because if we act same way 
towards
all wrong code in the driver, we could end up removing the whole i915.
So from my side I would nack it, at least until you have a code which handles
all of this better - I have no doubt you probably have some ideas in your mind, 
so lets be constructive at least and propose something better first.
This code doesn't cause any regressions, but still provides "some" support to 
DP MST DSC to say the least and even if that would be removed, if some of those 
users 
refer to me, I would probably then just point to this mail discussion everytime.

Stan


> 
> Cc: sta...@vger.kernel.org
> Cc: Vinod Govindapillai 
> Cc: Stanislav Lisovskiy 
> Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +---
>  1 file changed, 5 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 44c15d6faac4..d762f37fafb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   int min_bpp,
>   struct link_config_limits 
> *limits,
>   struct drm_connector_state 
> *conn_state,
> - int step,
> - bool dsc)
> + int step)
>  {
>   struct drm_atomic_state *state = crtc_state->uapi.state;
>   struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
>   drm_dbg_kms(>drm, "Trying bpp %d\n", bpp);
>  
> - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
> crtc_state, dsc);
> + ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
> crtc_state, false);
>   if (ret)
>   continue;
>  
> @@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   drm_dbg_kms(>drm, "failed finding vcpi slots:%d\n",
>   slots);
>   } else {
> - if (!dsc)
> - crtc_state->pipe_bpp = bpp;
> - else
> - crtc_state->dsc.compressed_bpp = bpp;
> - drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d dsc 
> %d\n", slots, bpp, dsc);
> + crtc_state->pipe_bpp = bpp;
> + drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d\n", 
> slots, bpp);
>   }
>  
>   return slots;
> @@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct 
> intel_encoder *encoder,
>  
>   slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, 
> limits->max_bpp,
>limits->min_bpp, limits,
> -  conn_state, 2 * 3, false);
> +  conn_state, 2 * 3);
>  
>   if (slots < 0)
>   return slots;
> @@ -173,99 +169,6 @@ static int 

Re: [PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-05-03 Thread Lisovskiy, Stanislav
On Tue, May 02, 2023 at 05:38:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> The MST DSC code has a myriad of issues:
> - Platform checks are wrong (MST+DSC is TGL+ only IIRC)
> - Return values of .mode_valid_ctx() are wrong
> - .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
>   of the code doesn't agree
> - compressed bpp calculations don't make sense
> - FEC handling needs to consider the entire link as opposed to just
>   the single stream. Currently FEC would only get enabled if the
>   first enabled stream is compressed. Also I'm not seeing anything
>   that would account for the FEC overhead in any bandwidth calculations
> - PPS SDP is only handled for the first stream via the dig_port
>   hooks, other streams will not be transmittitng any PPS SDPs
> - PPS SDP readout is missing (also missing for SST!)
> - VDSC readout is missing (also missing for SST!)
> 
> The FEC issues is really the big one since we have no way currently
> to apply such link wide configuration constraints. Changing that is
> going to require a much bigger rework of the higher level modeset
> .compute_config() logic. We will also need such a rework to properly
> distribute the available bandwidth across all the streams on the
> same link (which is a must to eg. enable deep color).

Hi Ville,

I'm not going to argue even that this code has some issues or not,
sure it has. However you are currently totally removing everything
without proposing what fixes have to be done or could have been done
instead.
We had multiple DP MST hubs which started to work because of this code,
also we had a public gitlab issues which were solved. So now they will loose
that support. I mean if this goes through, then I guess its up to you to 
handle this. Why not just fix instead of remove? Just add fixing patches on
top of those which remove the wrong code.
Now we are simply removing all that works at least,
instead of proposing how to fix, leaving no support at all. 
I have 2 MST hubs on my desk which won't work after this code is removed,
so if this patch goes through I recommend taking them.
But at least from my side, I think this is kinda unconstructive.

Stan


> 
> Cc: sta...@vger.kernel.org
> Cc: Vinod Govindapillai 
> Cc: Stanislav Lisovskiy 
> Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +---
>  1 file changed, 5 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 44c15d6faac4..d762f37fafb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   int min_bpp,
>   struct link_config_limits 
> *limits,
>   struct drm_connector_state 
> *conn_state,
> - int step,
> - bool dsc)
> + int step)
>  {
>   struct drm_atomic_state *state = crtc_state->uapi.state;
>   struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
>   drm_dbg_kms(>drm, "Trying bpp %d\n", bpp);
>  
> - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
> crtc_state, dsc);
> + ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
> crtc_state, false);
>   if (ret)
>   continue;
>  
> @@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
> intel_encoder *encoder,
>   drm_dbg_kms(>drm, "failed finding vcpi slots:%d\n",
>   slots);
>   } else {
> - if (!dsc)
> - crtc_state->pipe_bpp = bpp;
> - else
> - crtc_state->dsc.compressed_bpp = bpp;
> - drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d dsc 
> %d\n", slots, bpp, dsc);
> + crtc_state->pipe_bpp = bpp;
> + drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d\n", 
> slots, bpp);
>   }
>  
>   return slots;
> @@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct 
> intel_encoder *encoder,
>  
>   slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, 
> limits->max_bpp,
>limits->min_bpp, limits,
> -  conn_state, 2 * 3, false);
> +  conn_state, 2 * 3);
>  
>   if 

[PATCH 02/11] drm/i915/mst: Remove broken MST DSC support

2023-05-02 Thread Ville Syrjala
From: Ville Syrjälä 

The MST DSC code has a myriad of issues:
- Platform checks are wrong (MST+DSC is TGL+ only IIRC)
- Return values of .mode_valid_ctx() are wrong
- .mode_valid_ctx() assumes bigjoiner might be used, but ther rest
  of the code doesn't agree
- compressed bpp calculations don't make sense
- FEC handling needs to consider the entire link as opposed to just
  the single stream. Currently FEC would only get enabled if the
  first enabled stream is compressed. Also I'm not seeing anything
  that would account for the FEC overhead in any bandwidth calculations
- PPS SDP is only handled for the first stream via the dig_port
  hooks, other streams will not be transmittitng any PPS SDPs
- PPS SDP readout is missing (also missing for SST!)
- VDSC readout is missing (also missing for SST!)

The FEC issues is really the big one since we have no way currently
to apply such link wide configuration constraints. Changing that is
going to require a much bigger rework of the higher level modeset
.compute_config() logic. We will also need such a rework to properly
distribute the available bandwidth across all the streams on the
same link (which is a must to eg. enable deep color).

Cc: sta...@vger.kernel.org
Cc: Vinod Govindapillai 
Cc: Stanislav Lisovskiy 
Fixes: d51f25eb479a ("drm/i915: Add DSC support to MST path")
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 176 +---
 1 file changed, 5 insertions(+), 171 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 44c15d6faac4..d762f37fafb5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -72,8 +72,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
intel_encoder *encoder,
int min_bpp,
struct link_config_limits 
*limits,
struct drm_connector_state 
*conn_state,
-   int step,
-   bool dsc)
+   int step)
 {
struct drm_atomic_state *state = crtc_state->uapi.state;
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -104,7 +103,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
intel_encoder *encoder,
for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
drm_dbg_kms(>drm, "Trying bpp %d\n", bpp);
 
-   ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
crtc_state, dsc);
+   ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, 
crtc_state, false);
if (ret)
continue;
 
@@ -136,11 +135,8 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct 
intel_encoder *encoder,
drm_dbg_kms(>drm, "failed finding vcpi slots:%d\n",
slots);
} else {
-   if (!dsc)
-   crtc_state->pipe_bpp = bpp;
-   else
-   crtc_state->dsc.compressed_bpp = bpp;
-   drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d dsc 
%d\n", slots, bpp, dsc);
+   crtc_state->pipe_bpp = bpp;
+   drm_dbg_kms(>drm, "Got %d slots for pipe bpp %d\n", 
slots, bpp);
}
 
return slots;
@@ -157,7 +153,7 @@ static int intel_dp_mst_compute_link_config(struct 
intel_encoder *encoder,
 
slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, 
limits->max_bpp,
 limits->min_bpp, limits,
-conn_state, 2 * 3, false);
+conn_state, 2 * 3);
 
if (slots < 0)
return slots;
@@ -173,99 +169,6 @@ static int intel_dp_mst_compute_link_config(struct 
intel_encoder *encoder,
return 0;
 }
 
-static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
-   struct intel_crtc_state 
*crtc_state,
-   struct drm_connector_state 
*conn_state,
-   struct link_config_limits 
*limits)
-{
-   struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
-   struct intel_dp *intel_dp = _mst->primary->dp;
-   struct intel_connector *connector =
-   to_intel_connector(conn_state->connector);
-   struct drm_i915_private *i915 = to_i915(connector->base.dev);
-   const struct drm_display_mode *adjusted_mode =
-   _state->hw.adjusted_mode;
-   int slots = -EINVAL;
-   int i, num_bpc;
-   u8 dsc_bpc[3] = {0};
-   int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
-   u8 dsc_max_bpc;