Recall: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints
Liu, Wenjing would like to recall the message, "[PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints". ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints
On 8/20/19 3:12 PM, David Francis wrote: > The first step in MST DSC is checking MST endpoints > to see how DSC can be enabled > > Case 1: DP-to-DP peer device > if the branch immediately upstream has > - PDT = DP_PEER_DEVICE_DP_MST_BRANCHING (2) > - DPCD rev. >= DP 1.4 > - Exactly one input and one output > - The output has PDT = DP_PEER_DEVICE_SST_SINK (3) > > In this case, DSC could be possible either on the endpoint > or the peer device. Prefer the endpoint, which is possible if > - The endpoint has DP_DSC_DECOMPRESSION_IS_SUPPORTED bit set > - The endpoint has DP_FEC_CAPABLE bit set > - The peer device has DSC_PASSTHROUGH_CAPABILITY bit set (from DP v2.0) > > Otherwise, use the peer device > > Case 2: DP-to-HDMI peer device > If the output port has > - PDT = DP_PEER_DEVICE_DP_LEGACY_CONV (4) > - DPCD rev. >= DP 1.4 > - LDPS = true > - MCS = false > > In this case, DSC can only be attempted on the peer device > (the output port) > > Case 3: Virtual DP Sink (Internal Display Panel) > If the output port has > - DPCD rev. >= DP 1.4 > - port_num >= 8 > > In this case, DSC can only be attempted on the peer device > (the output port) > > Case 4: Synaptix Workaround > If the output has > - link DPCD rev. >= DP 1.4 > - link branch_dev_id = 0x90CC24 (Synaptix) > - There is exactly one branch device between the link and output > > In this case, DSC can be attempted, but only using the *link* > aux device's caps. This is a quirk. > > Test for these cases as modes are enumerated for an MST endpoint. > We cannot do this during link attach because the dc_sink object > will not have been created yet > > If no DSC is attempted, zero the DSC caps > > Cc: Wenjing Liu > Cc: Nikola Cornij > Signed-off-by: David Francis Questions and comments below... > --- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 123 +- > .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 3 + > 2 files changed, 125 insertions(+), 1 deletion(-) > > 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 16218a202b59..58571844f6d5 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 > @@ -25,6 +25,7 @@ > > #include > #include > +#include > #include "dm_services.h" > #include "amdgpu.h" > #include "amdgpu_dm.h" > @@ -189,6 +190,120 @@ static const struct drm_connector_funcs > dm_dp_mst_connector_funcs = { > .early_unregister = amdgpu_dm_mst_connector_early_unregister, > }; > > +bool is_virtual_dpcd(struct drm_dp_mst_port *port) > +{ > + struct drm_dp_mst_port *downstream_port; > + > + if (!port) > + return false; > + > + if (port->port_num >= 8 && > + port->dpcd_rev >= DP_DPCD_REV_14) { All these if statements should be aligned if possible. That's just formatting nitpicks though. > + /* Virtual DP Sink (Internal Display Panel) */ > + return true; > + } else if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV && > + !port->mcs && > + port->ldps && > + port->dpcd_rev >= DP_DPCD_REV_14) { > + /* DP-to-HDMI Protocol Converter */ > + return true; > + } else if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && > + port->mstb && > + port->dpcd_rev >= DP_DPCD_REV_14) { > + /* DP-to-DP */ > + if (port->mstb->num_ports == 2) { Can probably merge this if condition into the else if above. Will help cut down on line length below. > + list_for_each_entry(downstream_port, > &port->mstb->ports, next) { > + if (!downstream_port->input && > + downstream_port->pdt == > DP_PEER_DEVICE_SST_SINK) > + return true; > + } > + } > + } > + return false; > +} > + > +bool synaptix_workaround(struct amdgpu_dm_connector *aconnector) This probably needs a better name. This isn't applying a workaround for a specific device but returning true if it is a specific device. > +{ > + struct drm_dp_mst_port *port = aconnector->port; > + struct dc_link *link = aconnector->dc_sink->link; > + u8 down_stream_port_data; > + > + if (port->mgr->mst_primary == port->parent && > + link->dpcd_caps.branch_dev_id == 0x90CC24 && > + link->dpcd_caps.dpcd_rev.raw >= DP_DPCD_REV_14) { > + drm_dp_mst_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT, > &down_stream_port_data, 1); > + if ((down_stream_port_data & 7) != 3) > + return true; > + } > + return false; > +} > + > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > +static bool validate_dsc_caps_on_con
Re: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints
Hi, I was looking through patchwork and just noticed this series. I've been working on trying to rework a lot of the drm MST helpers (currently on hold for a short bit while I fix some other unrelated issues at work...), and one of the things we're currently missing with our helpers are helpers for handling checking the PBN of downstream sinks (which could be added to drm_dp_mst_atomic_check()). As well, it looks like you could also add a DRM helper for checking for DSC/FEC support on mstbs. Thoughts? On Tue, 2019-08-20 at 15:12 -0400, David Francis wrote: > The first step in MST DSC is checking MST endpoints > to see how DSC can be enabled > > Case 1: DP-to-DP peer device > if the branch immediately upstream has > - PDT = DP_PEER_DEVICE_DP_MST_BRANCHING (2) > - DPCD rev. >= DP 1.4 > - Exactly one input and one output > - The output has PDT = DP_PEER_DEVICE_SST_SINK (3) > > In this case, DSC could be possible either on the endpoint > or the peer device. Prefer the endpoint, which is possible if > - The endpoint has DP_DSC_DECOMPRESSION_IS_SUPPORTED bit set > - The endpoint has DP_FEC_CAPABLE bit set > - The peer device has DSC_PASSTHROUGH_CAPABILITY bit set (from DP v2.0) > > Otherwise, use the peer device > > Case 2: DP-to-HDMI peer device > If the output port has > - PDT = DP_PEER_DEVICE_DP_LEGACY_CONV (4) > - DPCD rev. >= DP 1.4 > - LDPS = true > - MCS = false > > In this case, DSC can only be attempted on the peer device > (the output port) > > Case 3: Virtual DP Sink (Internal Display Panel) > If the output port has > - DPCD rev. >= DP 1.4 > - port_num >= 8 > > In this case, DSC can only be attempted on the peer device > (the output port) > > Case 4: Synaptix Workaround > If the output has > - link DPCD rev. >= DP 1.4 > - link branch_dev_id = 0x90CC24 (Synaptix) > - There is exactly one branch device between the link and output > > In this case, DSC can be attempted, but only using the *link* > aux device's caps. This is a quirk. > > Test for these cases as modes are enumerated for an MST endpoint. > We cannot do this during link attach because the dc_sink object > will not have been created yet > > If no DSC is attempted, zero the DSC caps > > Cc: Wenjing Liu > Cc: Nikola Cornij > Signed-off-by: David Francis > --- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 123 +- > .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 3 + > 2 files changed, 125 insertions(+), 1 deletion(-) > > 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 16218a202b59..58571844f6d5 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 > @@ -25,6 +25,7 @@ > > #include > #include > +#include > #include "dm_services.h" > #include "amdgpu.h" > #include "amdgpu_dm.h" > @@ -189,6 +190,120 @@ static const struct drm_connector_funcs > dm_dp_mst_connector_funcs = { > .early_unregister = amdgpu_dm_mst_connector_early_unregister, > }; > > +bool is_virtual_dpcd(struct drm_dp_mst_port *port) > +{ > + struct drm_dp_mst_port *downstream_port; > + > + if (!port) > + return false; > + > + if (port->port_num >= 8 && > + port->dpcd_rev >= DP_DPCD_REV_14) { > + /* Virtual DP Sink (Internal Display Panel) */ > + return true; > + } else if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV && > + !port->mcs && > + port->ldps && > + port->dpcd_rev >= DP_DPCD_REV_14) { > + /* DP-to-HDMI Protocol Converter */ > + return true; > + } else if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && > + port->mstb && > + port->dpcd_rev >= DP_DPCD_REV_14) { > + /* DP-to-DP */ > + if (port->mstb->num_ports == 2) { > + list_for_each_entry(downstream_port, &port->mstb- > >ports, next) { > + if (!downstream_port->input && > + downstream_port->pdt == > DP_PEER_DEVICE_SST_SINK) > + return true; > + } > + } > + } > + return false; > +} > + > +bool synaptix_workaround(struct amdgpu_dm_connector *aconnector) > +{ > + struct drm_dp_mst_port *port = aconnector->port; > + struct dc_link *link = aconnector->dc_sink->link; > + u8 down_stream_port_data; > + > + if (port->mgr->mst_primary == port->parent && > + link->dpcd_caps.branch_dev_id == 0x90CC24 && > + link->dpcd_caps.dpcd_rev.raw >= DP_DPCD_REV_14) { > + drm_dp_mst_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT, > &down_stream_port_data, 1); > + if ((down_stream_port_data & 7) != 3) > + return true; > +
[PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints
The first step in MST DSC is checking MST endpoints to see how DSC can be enabled Case 1: DP-to-DP peer device if the branch immediately upstream has - PDT = DP_PEER_DEVICE_DP_MST_BRANCHING (2) - DPCD rev. >= DP 1.4 - Exactly one input and one output - The output has PDT = DP_PEER_DEVICE_SST_SINK (3) In this case, DSC could be possible either on the endpoint or the peer device. Prefer the endpoint, which is possible if - The endpoint has DP_DSC_DECOMPRESSION_IS_SUPPORTED bit set - The endpoint has DP_FEC_CAPABLE bit set - The peer device has DSC_PASSTHROUGH_CAPABILITY bit set (from DP v2.0) Otherwise, use the peer device Case 2: DP-to-HDMI peer device If the output port has - PDT = DP_PEER_DEVICE_DP_LEGACY_CONV (4) - DPCD rev. >= DP 1.4 - LDPS = true - MCS = false In this case, DSC can only be attempted on the peer device (the output port) Case 3: Virtual DP Sink (Internal Display Panel) If the output port has - DPCD rev. >= DP 1.4 - port_num >= 8 In this case, DSC can only be attempted on the peer device (the output port) Case 4: Synaptix Workaround If the output has - link DPCD rev. >= DP 1.4 - link branch_dev_id = 0x90CC24 (Synaptix) - There is exactly one branch device between the link and output In this case, DSC can be attempted, but only using the *link* aux device's caps. This is a quirk. Test for these cases as modes are enumerated for an MST endpoint. We cannot do this during link attach because the dc_sink object will not have been created yet If no DSC is attempted, zero the DSC caps Cc: Wenjing Liu Cc: Nikola Cornij Signed-off-by: David Francis --- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 123 +- .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 3 + 2 files changed, 125 insertions(+), 1 deletion(-) 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 16218a202b59..58571844f6d5 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 @@ -25,6 +25,7 @@ #include #include +#include #include "dm_services.h" #include "amdgpu.h" #include "amdgpu_dm.h" @@ -189,6 +190,120 @@ static const struct drm_connector_funcs dm_dp_mst_connector_funcs = { .early_unregister = amdgpu_dm_mst_connector_early_unregister, }; +bool is_virtual_dpcd(struct drm_dp_mst_port *port) +{ + struct drm_dp_mst_port *downstream_port; + + if (!port) + return false; + + if (port->port_num >= 8 && + port->dpcd_rev >= DP_DPCD_REV_14) { + /* Virtual DP Sink (Internal Display Panel) */ + return true; + } else if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV && + !port->mcs && + port->ldps && + port->dpcd_rev >= DP_DPCD_REV_14) { + /* DP-to-HDMI Protocol Converter */ + return true; + } else if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && + port->mstb && + port->dpcd_rev >= DP_DPCD_REV_14) { + /* DP-to-DP */ + if (port->mstb->num_ports == 2) { + list_for_each_entry(downstream_port, &port->mstb->ports, next) { + if (!downstream_port->input && + downstream_port->pdt == DP_PEER_DEVICE_SST_SINK) + return true; + } + } + } + return false; +} + +bool synaptix_workaround(struct amdgpu_dm_connector *aconnector) +{ + struct drm_dp_mst_port *port = aconnector->port; + struct dc_link *link = aconnector->dc_sink->link; + u8 down_stream_port_data; + + if (port->mgr->mst_primary == port->parent && + link->dpcd_caps.branch_dev_id == 0x90CC24 && + link->dpcd_caps.dpcd_rev.raw >= DP_DPCD_REV_14) { + drm_dp_mst_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT, &down_stream_port_data, 1); + if ((down_stream_port_data & 7) != 3) + return true; + } + return false; +} + +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT +static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector *aconnector) +{ + u8 upstream_dsc_caps[16] = { 0 }; + u8 endpoint_dsc_caps[16] = { 0 }; + u8 endpoint_fec_caps = 0; + struct dc_sink *dc_sink = aconnector->dc_sink; + struct drm_dp_mst_port *output_port = aconnector->port; + struct drm_dp_mst_port *immediate_upstream_port; + struct drm_dp_mst_port *fec_port; + + if (aconnector->port && aconnector->port->parent) + immediate_upstream_port = aconnector->port->parent->port_parent; + else + immediate_upstream_port = NULL; + + fec_port = immediate_upstream_