Recall: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints

2019-08-21 Thread Liu, Wenjing 
Liu, Wenjing would like to recall the message, "[PATCH v2 11/14] 
drm/amd/display: Validate DSC caps on MST endpoints".
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints

2019-08-21 Thread Kazlauskas, Nicholas 
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, 
> >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(>aux, DP_DOWNSTREAMPORT_PRESENT, 
> _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

Re: [PATCH v2 11/14] drm/amd/display: Validate DSC caps on MST endpoints

2019-08-20 Thread Lyude Paul 
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, >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(>aux, DP_DOWNSTREAMPORT_PRESENT,
> _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

2019-08-20 Thread David Francis 
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, 
>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(>aux, DP_DOWNSTREAMPORT_PRESENT, 
_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_port;
+