2022-08-05 21:16:01

by Hamza Mahfooz

[permalink] [raw]
Subject: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port

Currently, there is no way to identify if DSC pass-through can be
enabled and what aux DSC pass-through requests ought to be sent to. So,
add a variable to struct drm_dp_mst_port that keeps track of the
aforementioned information.

Signed-off-by: Hamza Mahfooz <[email protected]>
---
v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
include/drm/display/drm_dp.h | 1 +
include/drm/display/drm_dp_mst_helper.h | 3 +++
3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 67b3b9697da7..71915afd9892 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
/* Enpoint decompression with DP-to-DP peer device */
if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
(endpoint_fec & DP_FEC_CAPABLE) &&
- (upstream_dsc & 0x2) /* DSC passthrough */)
+ (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
+ port->passthrough_aux = &immediate_upstream_port->aux;
return &port->aux;
+ }

/* Virtual DPCD decompression with DP-to-DP peer device */
return &immediate_upstream_port->aux;
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 9e3aff7e68bb..4d0abe4c7ea9 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -239,6 +239,7 @@

#define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
# define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
+# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)

#define DP_DSC_REV 0x061
# define DP_DSC_MAJOR_MASK (0xf << 0)
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 10adec068b7f..4a39c95f8afd 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -86,6 +86,8 @@ struct drm_dp_vcpi {
* @next: link to next port on this branch device
* @aux: i2c aux transport to talk to device connected to this port, protected
* by &drm_dp_mst_topology_mgr.base.lock.
+ * @passthrough_aux: parent aux to which DSC pass-through requests should be
+ * sent, only set if DSC pass-through is possible.
* @parent: branch device parent of this port
* @vcpi: Virtual Channel Payload info for this port.
* @connector: DRM connector this port is connected to. Protected by
@@ -140,6 +142,7 @@ struct drm_dp_mst_port {
*/
struct drm_dp_mst_branch *mstb;
struct drm_dp_aux aux; /* i2c bus for this port? */
+ struct drm_dp_aux *passthrough_aux;
struct drm_dp_mst_branch *parent;

struct drm_dp_vcpi vcpi;
--
2.37.1


2022-08-05 21:18:25

by Hamza Mahfooz

[permalink] [raw]
Subject: [PATCH v2 3/3] drm/amd/display: implement DSC pass-through support

Currently, we only attempt to setup DSC at the virtual DPCD port,
however many modern displays come with DSC support and MST hubs can now
support DSC pass-through. So, to more optimally make use of the
available bandwidth, use DSC pass-through when possible by adding DSC
pass-through enablement support into the DSC enable sequence.

Signed-off-by: Hamza Mahfooz <[email protected]>
---
v2: print the correct type of port that we are writing to using dsc_aux
in dm_helpers_dp_write_dsc_enable().
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 45 +++++++++++++++++--
1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index a0154a5f7183..bd364d2cc4f7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -729,8 +729,14 @@ bool dm_helpers_dp_write_dsc_enable(
const struct dc_stream_state *stream,
bool enable)
{
- uint8_t enable_dsc = enable ? 1 : 0;
+ static const uint8_t DSC_DISABLE;
+ static const uint8_t DSC_DECODING = 0x01;
+ static const uint8_t DSC_PASSTHROUGH = 0x02;
+
struct amdgpu_dm_connector *aconnector;
+ struct drm_dp_mst_port *port;
+ uint8_t enable_dsc = enable ? DSC_DECODING : DSC_DISABLE;
+ uint8_t enable_passthrough = enable ? DSC_PASSTHROUGH : DSC_DISABLE;
uint8_t ret = 0;

if (!stream)
@@ -750,8 +756,39 @@ bool dm_helpers_dp_write_dsc_enable(
aconnector->dsc_aux, stream, enable_dsc);
#endif

- ret = drm_dp_dpcd_write(aconnector->dsc_aux, DP_DSC_ENABLE, &enable_dsc, 1);
- DC_LOG_DC("Send DSC %s to MST RX\n", enable_dsc ? "enable" : "disable");
+ port = aconnector->port;
+
+ if (enable) {
+ if (port->passthrough_aux) {
+ ret = drm_dp_dpcd_write(port->passthrough_aux,
+ DP_DSC_ENABLE,
+ &enable_passthrough, 1);
+ DC_LOG_DC("Sent DSC pass-through enable to virtual dpcd port, ret = %u\n",
+ ret);
+ }
+
+ ret = drm_dp_dpcd_write(aconnector->dsc_aux,
+ DP_DSC_ENABLE, &enable_dsc, 1);
+ DC_LOG_DC("Sent DSC decoding enable to %s port, ret = %u\n",
+ (port->passthrough_aux) ? "remote RX" :
+ "virtual dpcd",
+ ret);
+ } else {
+ ret = drm_dp_dpcd_write(aconnector->dsc_aux,
+ DP_DSC_ENABLE, &enable_dsc, 1);
+ DC_LOG_DC("Sent DSC decoding disable to %s port, ret = %u\n",
+ (port->passthrough_aux) ? "remote RX" :
+ "virtual dpcd",
+ ret);
+
+ if (port->passthrough_aux) {
+ ret = drm_dp_dpcd_write(port->passthrough_aux,
+ DP_DSC_ENABLE,
+ &enable_passthrough, 1);
+ DC_LOG_DC("Sent DSC pass-through disable to virtual dpcd port, ret = %u\n",
+ ret);
+ }
+ }
}

if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT || stream->signal == SIGNAL_TYPE_EDP) {
@@ -768,7 +805,7 @@ bool dm_helpers_dp_write_dsc_enable(
#endif
}

- return (ret > 0);
+ return ret;
}

bool dm_helpers_is_dp_sink_present(struct dc_link *link)
--
2.37.1

2022-08-05 21:24:26

by Hamza Mahfooz

[permalink] [raw]
Subject: [PATCH v2 2/3] drm/amd/display: consider DSC pass-through during mode validation

Add a mode validation routine for DSC pass-through. Both the link from
source to hub, and the link from hub to monitor are checked, according
to the current link training result and full pbn returned by enum path
resource sideband message.

Pick up the minimum value as the bandwidth bottleneck for the end to
end link bandwidth constraint, and check if the maximum DSC decompression
bandwidth can fit.

Co-authored-by: Fangzhi Zuo <[email protected]>
Signed-off-by: Hamza Mahfooz <[email protected]>
---
v2: use multi-line comments, use a temp variable in kbps_from_pbn() and
add a debug print statement in dm_dp_mst_is_port_support_mode().
---
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 79 +++++++++++++++++--
1 file changed, 74 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 2e74ccf7df5b..ef6c94cd852b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -36,6 +36,7 @@
#include "dm_helpers.h"

#include "dc_link_ddc.h"
+#include "dc_link_dp.h"
#include "ddc_service_types.h"
#include "dpcd_defs.h"

@@ -1388,17 +1389,85 @@ bool pre_validate_dsc(struct drm_atomic_state *state,

#endif

+static unsigned int kbps_from_pbn(unsigned int pbn)
+{
+ unsigned int kbps = pbn;
+
+ kbps *= (1000000 / PEAK_FACTOR_X1000);
+ kbps *= 8;
+ kbps *= 54;
+ kbps /= 64;
+
+ return kbps;
+}
+
+static bool is_dsc_common_config_possible(struct dc_stream_state *stream,
+ struct dc_dsc_bw_range *bw_range)
+{
+ struct dc_dsc_policy dsc_policy = {0};
+
+ dc_dsc_get_policy_for_timing(&stream->timing, 0, &dsc_policy);
+ dc_dsc_compute_bandwidth_range(stream->sink->ctx->dc->res_pool->dscs[0],
+ stream->sink->ctx->dc->debug.dsc_min_slice_height_override,
+ dsc_policy.min_target_bpp * 16,
+ dsc_policy.max_target_bpp * 16,
+ &stream->sink->dsc_caps.dsc_dec_caps,
+ &stream->timing, bw_range);
+
+ return bw_range->max_target_bpp_x16 && bw_range->min_target_bpp_x16;
+}
+
enum dc_status dm_dp_mst_is_port_support_mode(
struct amdgpu_dm_connector *aconnector,
struct dc_stream_state *stream)
{
+ struct dc_link_settings cur_link_settings;
+ unsigned int end_to_end_bw_in_kbps = 0;
+ unsigned int upper_link_bw_in_kbps = 0, down_link_bw_in_kbps = 0;
+ unsigned int max_compressed_bw_in_kbps = 0;
+ struct dc_dsc_bw_range bw_range = {0};
int bpp, pbn, branch_max_throughput_mps = 0;

- /* check if mode could be supported within fUll_pbn */
- bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
- pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
- if (pbn > aconnector->port->full_pbn)
- return DC_FAIL_BANDWIDTH_VALIDATE;
+ /*
+ * check if the mode could be supported if DSC pass-through is supported
+ * AND check if there enough bandwidth available to support the mode
+ * with DSC enabled.
+ */
+ if (is_dsc_common_config_possible(stream, &bw_range) &&
+ aconnector->port->passthrough_aux) {
+ mutex_lock(&aconnector->mst_mgr.lock);
+
+ cur_link_settings = stream->link->verified_link_cap;
+
+ upper_link_bw_in_kbps = dc_link_bandwidth_kbps(aconnector->dc_link,
+ &cur_link_settings
+ );
+ down_link_bw_in_kbps = kbps_from_pbn(aconnector->port->full_pbn);
+
+ /* pick the bottleneck */
+ end_to_end_bw_in_kbps = min(upper_link_bw_in_kbps,
+ down_link_bw_in_kbps);
+
+ mutex_unlock(&aconnector->mst_mgr.lock);
+
+ /*
+ * use the maximum dsc compression bandwidth as the required
+ * bandwidth for the mode
+ */
+ max_compressed_bw_in_kbps = bw_range.min_kbps;
+
+ if (end_to_end_bw_in_kbps < max_compressed_bw_in_kbps) {
+ DRM_DEBUG_DRIVER("Mode does not fit into DSC pass-through bandwidth validation\n");
+ return DC_FAIL_BANDWIDTH_VALIDATE;
+ }
+ } else {
+ /* check if mode could be supported within full_pbn */
+ bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3;
+ pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
+
+ if (pbn > aconnector->port->full_pbn)
+ return DC_FAIL_BANDWIDTH_VALIDATE;
+ }

/* check is mst dsc output bandwidth branch_overall_throughput_0_mps */
switch (stream->timing.pixel_encoding) {
--
2.37.1

2022-08-05 21:24:45

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port

lgtm!

Reviewed-by: Lyude Paul <[email protected]>

On Fri, 2022-08-05 at 17:13 -0400, Hamza Mahfooz wrote:
> Currently, there is no way to identify if DSC pass-through can be
> enabled and what aux DSC pass-through requests ought to be sent to. So,
> add a variable to struct drm_dp_mst_port that keeps track of the
> aforementioned information.
>
> Signed-off-by: Hamza Mahfooz <[email protected]>
> ---
> v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
> include/drm/display/drm_dp.h | 1 +
> include/drm/display/drm_dp_mst_helper.h | 3 +++
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 67b3b9697da7..71915afd9892 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
> /* Enpoint decompression with DP-to-DP peer device */
> if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
> (endpoint_fec & DP_FEC_CAPABLE) &&
> - (upstream_dsc & 0x2) /* DSC passthrough */)
> + (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
> + port->passthrough_aux = &immediate_upstream_port->aux;
> return &port->aux;
> + }
>
> /* Virtual DPCD decompression with DP-to-DP peer device */
> return &immediate_upstream_port->aux;
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index 9e3aff7e68bb..4d0abe4c7ea9 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -239,6 +239,7 @@
>
> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
> +# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)
>
> #define DP_DSC_REV 0x061
> # define DP_DSC_MAJOR_MASK (0xf << 0)
> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> index 10adec068b7f..4a39c95f8afd 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -86,6 +86,8 @@ struct drm_dp_vcpi {
> * @next: link to next port on this branch device
> * @aux: i2c aux transport to talk to device connected to this port, protected
> * by &drm_dp_mst_topology_mgr.base.lock.
> + * @passthrough_aux: parent aux to which DSC pass-through requests should be
> + * sent, only set if DSC pass-through is possible.
> * @parent: branch device parent of this port
> * @vcpi: Virtual Channel Payload info for this port.
> * @connector: DRM connector this port is connected to. Protected by
> @@ -140,6 +142,7 @@ struct drm_dp_mst_port {
> */
> struct drm_dp_mst_branch *mstb;
> struct drm_dp_aux aux; /* i2c bus for this port? */
> + struct drm_dp_aux *passthrough_aux;
> struct drm_dp_mst_branch *parent;
>
> struct drm_dp_vcpi vcpi;

--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

2022-08-09 15:21:42

by Hamza Mahfooz

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port

Hey Lyude,

On 2022-08-05 17:17, Lyude Paul wrote:
> lgtm!
>

Any chance you can apply this to drm-misc-next?

> Reviewed-by: Lyude Paul <[email protected]>
>
> On Fri, 2022-08-05 at 17:13 -0400, Hamza Mahfooz wrote:
>> Currently, there is no way to identify if DSC pass-through can be
>> enabled and what aux DSC pass-through requests ought to be sent to. So,
>> add a variable to struct drm_dp_mst_port that keeps track of the
>> aforementioned information.
>>
>> Signed-off-by: Hamza Mahfooz <[email protected]>
>> ---
>> v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
>> ---
>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
>> include/drm/display/drm_dp.h | 1 +
>> include/drm/display/drm_dp_mst_helper.h | 3 +++
>> 3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> index 67b3b9697da7..71915afd9892 100644
>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> @@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
>> /* Enpoint decompression with DP-to-DP peer device */
>> if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
>> (endpoint_fec & DP_FEC_CAPABLE) &&
>> - (upstream_dsc & 0x2) /* DSC passthrough */)
>> + (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
>> + port->passthrough_aux = &immediate_upstream_port->aux;
>> return &port->aux;
>> + }
>>
>> /* Virtual DPCD decompression with DP-to-DP peer device */
>> return &immediate_upstream_port->aux;
>> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
>> index 9e3aff7e68bb..4d0abe4c7ea9 100644
>> --- a/include/drm/display/drm_dp.h
>> +++ b/include/drm/display/drm_dp.h
>> @@ -239,6 +239,7 @@
>>
>> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
>> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
>> +# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)
>>
>> #define DP_DSC_REV 0x061
>> # define DP_DSC_MAJOR_MASK (0xf << 0)
>> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
>> index 10adec068b7f..4a39c95f8afd 100644
>> --- a/include/drm/display/drm_dp_mst_helper.h
>> +++ b/include/drm/display/drm_dp_mst_helper.h
>> @@ -86,6 +86,8 @@ struct drm_dp_vcpi {
>> * @next: link to next port on this branch device
>> * @aux: i2c aux transport to talk to device connected to this port, protected
>> * by &drm_dp_mst_topology_mgr.base.lock.
>> + * @passthrough_aux: parent aux to which DSC pass-through requests should be
>> + * sent, only set if DSC pass-through is possible.
>> * @parent: branch device parent of this port
>> * @vcpi: Virtual Channel Payload info for this port.
>> * @connector: DRM connector this port is connected to. Protected by
>> @@ -140,6 +142,7 @@ struct drm_dp_mst_port {
>> */
>> struct drm_dp_mst_branch *mstb;
>> struct drm_dp_aux aux; /* i2c bus for this port? */
>> + struct drm_dp_aux *passthrough_aux;
>> struct drm_dp_mst_branch *parent;
>>
>> struct drm_dp_vcpi vcpi;
>

--
Hamza

2022-08-09 22:07:12

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port

Ah yes of course! Probably should have asked when I gave the r-b :). Also,
just so patchwork actually catches it I will say the magic incantation:

Reviewed-by: Lyude Paul <[email protected]>

Do we want to merge just this patch to drm-misc-next, or do you want to merge
the whole series through there? If you'd rather just merge this through amd's
branch I'm fine with that as well

On Tue, 2022-08-09 at 11:15 -0400, Hamza Mahfooz wrote:
> Hey Lyude,
>
> On 2022-08-05 17:17, Lyude Paul wrote:
> > lgtm!
> >
>
> Any chance you can apply this to drm-misc-next?
>
> > Reviewed-by: Lyude Paul <[email protected]>
> >
> > On Fri, 2022-08-05 at 17:13 -0400, Hamza Mahfooz wrote:
> > > Currently, there is no way to identify if DSC pass-through can be
> > > enabled and what aux DSC pass-through requests ought to be sent to. So,
> > > add a variable to struct drm_dp_mst_port that keeps track of the
> > > aforementioned information.
> > >
> > > Signed-off-by: Hamza Mahfooz <[email protected]>
> > > ---
> > > v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
> > > ---
> > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
> > > include/drm/display/drm_dp.h | 1 +
> > > include/drm/display/drm_dp_mst_helper.h | 3 +++
> > > 3 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > index 67b3b9697da7..71915afd9892 100644
> > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > @@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
> > > /* Enpoint decompression with DP-to-DP peer device */
> > > if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
> > > (endpoint_fec & DP_FEC_CAPABLE) &&
> > > - (upstream_dsc & 0x2) /* DSC passthrough */)
> > > + (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
> > > + port->passthrough_aux = &immediate_upstream_port->aux;
> > > return &port->aux;
> > > + }
> > >
> > > /* Virtual DPCD decompression with DP-to-DP peer device */
> > > return &immediate_upstream_port->aux;
> > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> > > index 9e3aff7e68bb..4d0abe4c7ea9 100644
> > > --- a/include/drm/display/drm_dp.h
> > > +++ b/include/drm/display/drm_dp.h
> > > @@ -239,6 +239,7 @@
> > >
> > > #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
> > > # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
> > > +# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)
> > >
> > > #define DP_DSC_REV 0x061
> > > # define DP_DSC_MAJOR_MASK (0xf << 0)
> > > diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> > > index 10adec068b7f..4a39c95f8afd 100644
> > > --- a/include/drm/display/drm_dp_mst_helper.h
> > > +++ b/include/drm/display/drm_dp_mst_helper.h
> > > @@ -86,6 +86,8 @@ struct drm_dp_vcpi {
> > > * @next: link to next port on this branch device
> > > * @aux: i2c aux transport to talk to device connected to this port, protected
> > > * by &drm_dp_mst_topology_mgr.base.lock.
> > > + * @passthrough_aux: parent aux to which DSC pass-through requests should be
> > > + * sent, only set if DSC pass-through is possible.
> > > * @parent: branch device parent of this port
> > > * @vcpi: Virtual Channel Payload info for this port.
> > > * @connector: DRM connector this port is connected to. Protected by
> > > @@ -140,6 +142,7 @@ struct drm_dp_mst_port {
> > > */
> > > struct drm_dp_mst_branch *mstb;
> > > struct drm_dp_aux aux; /* i2c bus for this port? */
> > > + struct drm_dp_aux *passthrough_aux;
> > > struct drm_dp_mst_branch *parent;
> > >
> > > struct drm_dp_vcpi vcpi;
> >
>

--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

2022-08-10 14:09:27

by Hamza Mahfooz

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port


On 2022-08-09 18:01, Lyude Paul wrote:
> Ah yes of course! Probably should have asked when I gave the r-b :). Also,
> just so patchwork actually catches it I will say the magic incantation:
>
> Reviewed-by: Lyude Paul <[email protected]>
>
> Do we want to merge just this patch to drm-misc-next, or do you want to merge
> the whole series through there? If you'd rather just merge this through amd's
> branch I'm fine with that as well

In that case, it is preferable to have all of the patches in this series
to get merged through amd's branch.

>
> On Tue, 2022-08-09 at 11:15 -0400, Hamza Mahfooz wrote:
>> Hey Lyude,
>>
>> On 2022-08-05 17:17, Lyude Paul wrote:
>>> lgtm!
>>>
>>
>> Any chance you can apply this to drm-misc-next?
>>
>>> Reviewed-by: Lyude Paul <[email protected]>
>>>
>>> On Fri, 2022-08-05 at 17:13 -0400, Hamza Mahfooz wrote:
>>>> Currently, there is no way to identify if DSC pass-through can be
>>>> enabled and what aux DSC pass-through requests ought to be sent to. So,
>>>> add a variable to struct drm_dp_mst_port that keeps track of the
>>>> aforementioned information.
>>>>
>>>> Signed-off-by: Hamza Mahfooz <[email protected]>
>>>> ---
>>>> v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
>>>> ---
>>>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
>>>> include/drm/display/drm_dp.h | 1 +
>>>> include/drm/display/drm_dp_mst_helper.h | 3 +++
>>>> 3 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>>>> index 67b3b9697da7..71915afd9892 100644
>>>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>>>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>>>> @@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
>>>> /* Enpoint decompression with DP-to-DP peer device */
>>>> if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
>>>> (endpoint_fec & DP_FEC_CAPABLE) &&
>>>> - (upstream_dsc & 0x2) /* DSC passthrough */)
>>>> + (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
>>>> + port->passthrough_aux = &immediate_upstream_port->aux;
>>>> return &port->aux;
>>>> + }
>>>>
>>>> /* Virtual DPCD decompression with DP-to-DP peer device */
>>>> return &immediate_upstream_port->aux;
>>>> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
>>>> index 9e3aff7e68bb..4d0abe4c7ea9 100644
>>>> --- a/include/drm/display/drm_dp.h
>>>> +++ b/include/drm/display/drm_dp.h
>>>> @@ -239,6 +239,7 @@
>>>>
>>>> #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
>>>> # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
>>>> +# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)
>>>>
>>>> #define DP_DSC_REV 0x061
>>>> # define DP_DSC_MAJOR_MASK (0xf << 0)
>>>> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
>>>> index 10adec068b7f..4a39c95f8afd 100644
>>>> --- a/include/drm/display/drm_dp_mst_helper.h
>>>> +++ b/include/drm/display/drm_dp_mst_helper.h
>>>> @@ -86,6 +86,8 @@ struct drm_dp_vcpi {
>>>> * @next: link to next port on this branch device
>>>> * @aux: i2c aux transport to talk to device connected to this port, protected
>>>> * by &drm_dp_mst_topology_mgr.base.lock.
>>>> + * @passthrough_aux: parent aux to which DSC pass-through requests should be
>>>> + * sent, only set if DSC pass-through is possible.
>>>> * @parent: branch device parent of this port
>>>> * @vcpi: Virtual Channel Payload info for this port.
>>>> * @connector: DRM connector this port is connected to. Protected by
>>>> @@ -140,6 +142,7 @@ struct drm_dp_mst_port {
>>>> */
>>>> struct drm_dp_mst_branch *mstb;
>>>> struct drm_dp_aux aux; /* i2c bus for this port? */
>>>> + struct drm_dp_aux *passthrough_aux;
>>>> struct drm_dp_mst_branch *parent;
>>>>
>>>> struct drm_dp_vcpi vcpi;
>>>
>>
>

--
Hamza

2022-08-10 21:26:22

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drm/dp_mst: add passthrough_aux to struct drm_dp_mst_port

On Wed, 2022-08-10 at 09:23 -0400, Hamza Mahfooz wrote:
> On 2022-08-09 18:01, Lyude Paul wrote:
> > Ah yes of course! Probably should have asked when I gave the r-b :). Also,
> > just so patchwork actually catches it I will say the magic incantation:
> >
> > Reviewed-by: Lyude Paul <[email protected]>
> >
> > Do we want to merge just this patch to drm-misc-next, or do you want to merge
> > the whole series through there? If you'd rather just merge this through amd's
> > branch I'm fine with that as well
>
> In that case, it is preferable to have all of the patches in this series
> to get merged through amd's branch.\

Sounds totally fine to me! Just make sure to run it by Harry or another amdgpu
maintainer first, and then you should be good to go.

>
> >
> > On Tue, 2022-08-09 at 11:15 -0400, Hamza Mahfooz wrote:
> > > Hey Lyude,
> > >
> > > On 2022-08-05 17:17, Lyude Paul wrote:
> > > > lgtm!
> > > >
> > >
> > > Any chance you can apply this to drm-misc-next?
> > >
> > > > Reviewed-by: Lyude Paul <[email protected]>
> > > >
> > > > On Fri, 2022-08-05 at 17:13 -0400, Hamza Mahfooz wrote:
> > > > > Currently, there is no way to identify if DSC pass-through can be
> > > > > enabled and what aux DSC pass-through requests ought to be sent to. So,
> > > > > add a variable to struct drm_dp_mst_port that keeps track of the
> > > > > aforementioned information.
> > > > >
> > > > > Signed-off-by: Hamza Mahfooz <[email protected]>
> > > > > ---
> > > > > v2: define DP_DSC_PASSTHROUGH_IS_SUPPORTED
> > > > > ---
> > > > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++-
> > > > > include/drm/display/drm_dp.h | 1 +
> > > > > include/drm/display/drm_dp_mst_helper.h | 3 +++
> > > > > 3 files changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > index 67b3b9697da7..71915afd9892 100644
> > > > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > > > > @@ -5921,8 +5921,10 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
> > > > > /* Enpoint decompression with DP-to-DP peer device */
> > > > > if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
> > > > > (endpoint_fec & DP_FEC_CAPABLE) &&
> > > > > - (upstream_dsc & 0x2) /* DSC passthrough */)
> > > > > + (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
> > > > > + port->passthrough_aux = &immediate_upstream_port->aux;
> > > > > return &port->aux;
> > > > > + }
> > > > >
> > > > > /* Virtual DPCD decompression with DP-to-DP peer device */
> > > > > return &immediate_upstream_port->aux;
> > > > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> > > > > index 9e3aff7e68bb..4d0abe4c7ea9 100644
> > > > > --- a/include/drm/display/drm_dp.h
> > > > > +++ b/include/drm/display/drm_dp.h
> > > > > @@ -239,6 +239,7 @@
> > > > >
> > > > > #define DP_DSC_SUPPORT 0x060 /* DP 1.4 */
> > > > > # define DP_DSC_DECOMPRESSION_IS_SUPPORTED (1 << 0)
> > > > > +# define DP_DSC_PASSTHROUGH_IS_SUPPORTED (1 << 1)
> > > > >
> > > > > #define DP_DSC_REV 0x061
> > > > > # define DP_DSC_MAJOR_MASK (0xf << 0)
> > > > > diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> > > > > index 10adec068b7f..4a39c95f8afd 100644
> > > > > --- a/include/drm/display/drm_dp_mst_helper.h
> > > > > +++ b/include/drm/display/drm_dp_mst_helper.h
> > > > > @@ -86,6 +86,8 @@ struct drm_dp_vcpi {
> > > > > * @next: link to next port on this branch device
> > > > > * @aux: i2c aux transport to talk to device connected to this port, protected
> > > > > * by &drm_dp_mst_topology_mgr.base.lock.
> > > > > + * @passthrough_aux: parent aux to which DSC pass-through requests should be
> > > > > + * sent, only set if DSC pass-through is possible.
> > > > > * @parent: branch device parent of this port
> > > > > * @vcpi: Virtual Channel Payload info for this port.
> > > > > * @connector: DRM connector this port is connected to. Protected by
> > > > > @@ -140,6 +142,7 @@ struct drm_dp_mst_port {
> > > > > */
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > struct drm_dp_aux aux; /* i2c bus for this port? */
> > > > > + struct drm_dp_aux *passthrough_aux;
> > > > > struct drm_dp_mst_branch *parent;
> > > > >
> > > > > struct drm_dp_vcpi vcpi;
> > > >
> > >
> >
>

--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

2022-08-11 11:58:09

by Anders Roxell

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] drm/amd/display: consider DSC pass-through during mode validation


On 2022-08-05 17:13, Hamza Mahfooz wrote:
> Add a mode validation routine for DSC pass-through. Both the link from
> source to hub, and the link from hub to monitor are checked, according
> to the current link training result and full pbn returned by enum path
> resource sideband message.
>
> Pick up the minimum value as the bandwidth bottleneck for the end to
> end link bandwidth constraint, and check if the maximum DSC decompression
> bandwidth can fit.
>
> Co-authored-by: Fangzhi Zuo <[email protected]>
> Signed-off-by: Hamza Mahfooz <[email protected]>

Hi,

Building an arm64 allmodconfig kernel from todays next tag, next-20220811.
I can see the following build error.

ERROR: modpost: "dc_dsc_compute_bandwidth_range" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: modpost: "dc_dsc_get_policy_for_timing" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!

When I reverted this patch I was able to build an arm64 allmodconfig
kernel.


Cheers,
Anders