Re: [PATCH v9 6/6] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux
fec_port = immediate_upstream_port;
>> + while (fec_port) {
>> + /*
>> + * Each physical link (i.e. not a virtual port) between the
>> + * output and the primary device must support FEC
>> + */
>> + if (!drm_dp_mst_is_virtual_dpcd(fec_port) &&
>> + !fec_port->fec_capable)
>> + return NULL;
>> +
>> + fec_port = fec_port->parent->port_parent;
>> + }
>> +
>> + /* DP-to-DP peer device */
>> + if (drm_dp_mst_is_virtual_dpcd(immediate_upstream_port)) {
>> + u8 upstream_dsc;
>> + u8 endpoint_dsc;
>> + u8 endpoint_fec;
>> +
>> + if (drm_dp_dpcd_read(&port->aux,
>> + DP_DSC_SUPPORT, &endpoint_dsc, 1) < 0)
>> + return NULL;
>> + if (drm_dp_dpcd_read(&port->aux,
>> + DP_FEC_CAPABILITY, &endpoint_fec, 1) < 0)
>> + return NULL;
>> + if (drm_dp_dpcd_read(&immediate_upstream_port->aux,
>> + DP_DSC_SUPPORT, &upstream_dsc, 1) < 0)
>> + return NULL;
>> +
>> + /* 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 */)
>> + return &port->aux;
>> +
>> + /* Virtual DPCD decompression with DP-to-DP peer device */
>> + return &immediate_upstream_port->aux;
>> + }
>> +
>> + /* Virtual DPCD decompression with DP-to-HDMI or Virtual DP Sink */
>> + if (drm_dp_mst_is_virtual_dpcd(port))
>> + return &port->aux;
>> +
>> + /*
>> + * Synaptics quirk
>> + * Applies to ports for which:
>> + * - Physical aux has Synaptics OUI
>> + * - DPv1.4 or higher
>> + * - Port is on primary branch device
>> + * - Not a VGA adapter (DP_DWN_STRM_PORT_TYPE_ANALOG)
>> + */
>> + if (!drm_dp_read_desc(port->mgr->aux, desc, true))
>> + return false;
>> +
>> + if (drm_dp_has_quirk(desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD &&
>> + port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14) &&
>> + port->parent == port->mgr->mst_primary) {
>> + u8 downstreamport;
>> +
>> + drm_dp_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT,
>> + &downstreamport, 1);
>> +
>> + if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) &&
>> +((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK)
>> + != DP_DWN_STRM_PORT_TYPE_ANALOG))
>> + return port->mgr->aux;
>> + }
>
>I would've put the quirk definition from patch 5 and the quirk handling
>above together into a separate patch. The current patch would be about
>how things should work in theory, and another patch on top would be
>about how reality interferes with theory, and you have to quirk stuff to
>make it actually work.
>
>The whole point about splitting the quirk off was that it's a separate
>functional change. What if the quirk turns out to be a mistake? Just
>revert the whole thing. That would be impossible with this split.
>
>I don't know if I care enough to insist. *shrug*
>
No, that's a good point. Fixing this for v10
>
>BR,
>Jani.
>
>
>
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port);
>> diff --git a/include/drm/drm_dp_mst_helper.h
>> b/include/drm/drm_dp_mst_helper.h
>> index f113ae04fa88..4cf738545dfb 100644
>> --- a/include/drm/drm_dp_mst_helper.h
>> +++ b/include/drm/drm_dp_mst_helper.h
>> @@ -673,6 +673,8 @@ int __must_check drm_dp_mst_atomic_check(struct
>> drm_atomic_state *state);
>> void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port);
>> void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);
>>
>> +struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port
>> *port);
>> +
>> extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
>>
>> /**
>
>--
Re: [PATCH v8 6/6] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux
Received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu, giving
me permission to mark this patch
Reviewed-by: Wenjing Liu
From: David Francis
Sent: August 26, 2019 2:05 PM
To: dri-devel@lists.freedesktop.org
Cc: Francis, David; Lyude Paul; Jani Nikula
Subject: [PATCH v8 6/6] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux
Add drm_dp_mst_dsc_aux_for_port. To enable DSC, the DSC_ENABLED
register might have to be written on the leaf port's DPCD,
its parent's DPCD, or the MST manager's DPCD. This function
finds the correct aux for the job.
As part of this, add drm_dp_mst_is_virtual_dpcd. Virtual DPCD
is a DP feature new in DP v1.4, which exposes certain DPCD
registers on virtual ports.
v2: Remember to unlock mutex on all paths
Cc: Lyude Paul
Cc: Jani Nikula
Signed-off-by: David Francis
---
drivers/gpu/drm/drm_dp_mst_topology.c | 152 ++
include/drm/drm_dp_mst_helper.h | 2 +
2 files changed, 154 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 502923c24450..1fee92bd51f7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4150,3 +4150,155 @@ static void drm_dp_mst_unregister_i2c_bus(struct
drm_dp_aux *aux)
{
i2c_del_adapter(&aux->ddc);
}
+
+/**
+ * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device
+ * @port: The port to check
+ *
+ * A single physical MST hub object can be represented in the topology
+ * by multiple branches, with virtual ports between those branches.
+ *
+ * As of DP1.4, An MST hub with internal (virtual) ports must expose
+ * certain DPCD registers over those ports. See sections 2.6.1.1.1
+ * and 2.6.1.1.2 of Display Port specification v1.4 for details.
+ *
+ * May acquire mgr->lock
+ *
+ * Returns:
+ * true if the port is a virtual DP peer device, false otherwise
+ */
+static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_port *downstream_port;
+
+ if (!port)
+ return false;
+
+ /* Virtual DP Sink (Internal Display Panel) */
+ if (port->port_num >= 8 && port->dpcd_rev >= DP_DPCD_REV_14)
+ return true;
+
+ /* DP-to-HDMI Protocol Converter */
+ if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV &&
+ !port->mcs &&
+ port->ldps &&
+ port->dpcd_rev >= DP_DPCD_REV_14)
+ return true;
+
+ /* DP-to-DP */
+ mutex_lock(&port->mgr->lock);
+ if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
+ port->mstb &&
+ port->dpcd_rev >= DP_DPCD_REV_14 &&
+ port->mstb->num_ports == 2) {
+ list_for_each_entry(downstream_port, &port->mstb->ports, next) {
+ if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK &&
+ !downstream_port->input) {
+ mutex_unlock(&port->mgr->lock);
+ return true;
+ }
+ }
+ }
+ mutex_unlock(&port->mgr->lock);
+
+ return false;
+}
+
+/**
+ * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC
+ * @port: The port to check. A leaf of the MST tree with an attached display.
+ *
+ * Depending on the situation, DSC may be enabled via the endpoint aux,
+ * the immediately upstream aux, or the connector's physical aux.
+ *
+ * This is both the correct aux to read DSC_CAPABILITY and the
+ * correct aux to write DSC_ENABLED.
+ *
+ * This operation can be expensive (up to four aux reads), so
+ * the caller should cache the return.
+ *
+ * Returns:
+ * NULL if DSC cannot be enabled on this port, otherwise the aux device
+ */
+struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_port *immediate_upstream_port;
+ struct drm_dp_mst_port *fec_port;
+ struct drm_dp_desc *desc = NULL;
+
+ if (port && port->parent)
+ immediate_upstream_port = port->parent->port_parent;
+ else
+ immediate_upstream_port = NULL;
+
+ fec_port = immediate_upstream_port;
+ while (fec_port) {
+ /*
+* Each physical link (i.e. not a virtual port) between the
+* output and the primary device must support FEC
+*/
+ if (!drm_dp_mst_is_virtual_dpcd(fec_port) &&
+ !fec_port->fec_capable)
+ return NULL;
+
+ fec_port = fec_port->parent->port_parent;
+ }
+
+
Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs
On 2019-08-26 2:05 p.m., David Francis wrote:
>> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
>> support virtual DPCD registers, but do support DSC.
>> The DSC caps can be read from the physical aux,
>> like in SST DSC. These hubs have many different
>> DEVICE_IDs. Add a new quirk to detect this case.
>>
>> Cc: Lyude Paul
>> Cc: Jani Nikula
>> Signed-off-by: David Francis
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>> include/drm/drm_dp_helper.h | 7 +++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c
>> index 2cc21eff4cf3..fc39323e7d52 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>>{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
>> BIT(DP_DPCD_QUIRK_NO_PSR) },
>>/* CH7511 seems to leave SINK_COUNT zeroed */
>>{ OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'),
>> false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
>> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true,
>> BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
>
>This seems to be generic OUI for Synaptics [1]. Could this cause us to
>cast the net too wide?
>
>Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance
>Synaptics is fixing this in the future and won't require the quirk.
>
>[1] https://aruljohn.com/mac/vendor/Synaptics
>
>
>Harry
It's not pretty, but
- Currently the net of "all Synaptics devices with rev>DP1.4" catches every
device we care about and none we don't
- If a future Synaptics device supports virtual DPCD properly, we will check
for that first and never resort to the workaround (see patch 6/6)
- We don't know any of the properties of any hypothetical future Synaptics
hardware, so we can't create cases for them now
Also, received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu,
giving me permission o mark this patch
Reviewed-by: Wenjing Liu
>
>> };
>>
>> #undef OUI
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index 8364502f92cf..a1331b08705f 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk {
>> * The driver should ignore SINK_COUNT during detection.
>> */
>>DP_DPCD_QUIRK_NO_SINK_COUNT,
>> + /**
>> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD:
>> + *
>> + * The device supports MST DSC despite not supporting Virtual DPCD.
>> + * The DSC caps can be read from the physical aux instead.
>> + */
>> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
>> };
>>
>> /**
From: Wentland, Harry
Sent: August 26, 2019 3:05 PM
To: Francis, David; dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs
On 2019-08-26 2:05 p.m., David Francis wrote:
> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
> support virtual DPCD registers, but do support DSC.
> The DSC caps can be read from the physical aux,
> like in SST DSC. These hubs have many different
> DEVICE_IDs. Add a new quirk to detect this case.
>
> Cc: Lyude Paul
> Cc: Jani Nikula
> Signed-off-by: David Francis
> ---
> drivers/gpu/drm/drm_dp_helper.c | 2 ++
> include/drm/drm_dp_helper.h | 7 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 2cc21eff4cf3..fc39323e7d52 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
> BIT(DP_DPCD_QUIRK_NO_PSR) },
> /* CH7511 seems to leave SINK_COUNT zeroed */
> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'),
> false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true,
> BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
This seems to be generic OUI for Synaptics [1]. Could this cause us to
cast the net too wide?
Even if we check that it's DP_DPCD_REV >= 1.4 there's
Re: [PATCH v4 0/5] MST DSC support in drm-mst
Adding DSC functionality to drm_dp_mst_atomic_check() is a good idea. However, until amdgpu switches over to that system, I wouldn't be able to test those changes. Making that switch is on our TODO list, and it would fix a number of problems with our current MST implementation, but it's going to be a major rewrite. MST DSC hardware is already on the market. It would be expedient to merge the patches we need for Navi support sooner and update drm_dp_mst_atomic_check when we're able to test it. David Francis From: Lyude Paul Sent: August 22, 2019 8:03 PM To: Francis, David; dri-devel@lists.freedesktop.org; Manasi Navare Subject: Re: [PATCH v4 0/5] MST DSC support in drm-mst OK-done reviewing, but there's some stuff missing here. The PBN bandwidth checks in https://patchwork.freedesktop.org/patch/325604/?series=65423&rev=3 need to be moved into drm_dp_mst_atomic_check(), along with moving amdgpu over to using drm_dp_mst_atomic_check(). Doing so will also give us PBN bandwidth checks in both i915 and nouveau as well, and keeps the bandwidth calculation where it should be. Additionally, you still need to move the code here into an MST atomic helper or drm_dp_mst_atomic_check() as well: https://patchwork.freedesktop.org/patch/325611/?series=65423&rev=3 Unless I'm mistaken, adding each CRTC which has a connector whose PBN requires recalculation into the atomic state is something every DRM driver is going to need to do. And, you can do this more easily by adding PBN information into drm_dp_mst_topology_state. Yes-it's a lot of locks, but since every connector in an MST topology is sharing the bandwidth on the same link it's kind of expected. I already know I'm going to have to do basically the same thing with every driver once I've got the time to actually implement fallback link rate retraining with MST topologies. If you need help figuring out how to structure this in a way that works for all drivers, I'm willing to help and I'm sure Manasi is as well. On Thu, 2019-08-22 at 09:57 -0400, David Francis wrote: > Add necessary support for MST DSC. > (Display Stream COmpression over Multi-Stream Transport) > > v4: Split patchset and rebase onto drm-tip > > David Francis (5): > drm/dp-mst: Add PBN calculation for DSC modes > drm/dp-mst: Parse FEC capability on MST ports > drm/dp-mst: Add MST support to DP DPCD R/W functions > drm/dp-mst: Fill branch->num_ports > drm/dp-mst: Add helpers for querying and enabling MST DSC > > drivers/gpu/drm/drm_dp_aux_dev.c | 12 +- > drivers/gpu/drm/drm_dp_helper.c | 10 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 243 ++ > include/drm/drm_dp_mst_helper.h | 8 +- > 4 files changed, 260 insertions(+), 13 deletions(-) > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/16] Display Stream Compression (DSC) for AMD Navi
I was building against amd-staging-drm-next (commit e4a67e6cf14c). v4 will contain just the drm-mst patches and will apply on latest drm-tip/drm-tip (commit 018886de4726) From: Lyude Paul Sent: August 21, 2019 5:20 PM To: Francis, David; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org Subject: Re: [PATCH v3 00/16] Display Stream Compression (DSC) for AMD Navi What branch does this patch series actually apply to? I've been trying to apply this locally, but it doesn't appear to apply against drm-tip/drm-tip, amdgpu-next/drm-next, or origin (e.g. kernel.org) /master. Is there any chance we could have this go against drm-tip instead (and even better, split out the DRM-specific bits into their own patch series?) On Wed, 2019-08-21 at 16:01 -0400, David Francis wrote: > This patchset enables Display Stream Compression (DSC) on DP > connectors on Navi ASICs, both SST and DSC. > > 8k60 and 4k144 support requires ODM combine, an AMD internal > feature that may be a bit buggy right now. > > Patches 1 through 5 enable DSC for SST. Most of the work was > already done in the Navi promotion patches; this just hooks > it up to the atomic interface. The first two reverts are of temporary > changes to block off DSC. The third is of a commit that was > accidentally promoted twice. The fourth and last revert fixes a > potential issue with ODM combine. > > Patches 6, 7 and 8 are fixes for bugs that would be exposed by > MST DSC. Patches 6 and 7 add and use a new DRM helper for MST > calculations. Patch 8 fixes a silly use-uninitialized > > Patches 9, 10, and 11 are small DRM changes required for DSC MST: > FEC, a new bit in the standard; MST DPCD from drivers; and > a previously uninitialized variable. > > Patches 12 through 16 are the DSC MST policy itself. Patch 12 > adds DSC aux access helpers to DRM, and patches 13 and 14 make > use of those helpers. Patch 15 deals with dividing bandwidth > fairly between multiple streams, and patch 16 ensures > that MST CRTC that may change DSC config are reprogrammed > > v2: Updating patches 6 and 14 in respoinse to Nick's feedback > v3: Add return value to patch 6 and split it (now patches 6 & 7) > New patch 10 adding MST DPCD read/write support > Minor fix (num_ports--) to patch 11 > Add DRM helpers (patch 12) > > David Francis (16): > Revert "drm/amd/display: skip dsc config for navi10 bring up" > Revert "drm/amd/display: navi10 bring up skip dsc encoder config" > Revert "drm/amd/display: add global master update lock for DCN2" > Revert "drm/amd/display: Fix underscan not using proper scaling" > drm/amd/display: Enable SST DSC in DM > drm/dp-mst: Add PBN calculation for DSC modes > drm/amd/display: Use correct helpers to compute timeslots > drm/amd/display: Initialize DSC PPS variables to 0 > drm/dp-mst: Parse FEC capability on MST ports > drm/dp-mst: Add MST support to DP DPCD R/W functions > drm/dp-mst: Fill branch->num_ports > drm/dp-mst: Add helpers for querying and enabling MST DSC > drm/amd/display: Validate DSC caps on MST endpoints > drm/amd/display: Write DSC enable to MST DPCD > drm/amd/display: MST DSC compute fair share > drm/amd/display: Trigger modesets on MST DSC connectors > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 113 - > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 33 +- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 402 +- > .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 4 + > drivers/gpu/drm/amd/display/dc/core/dc.c | 12 +- > .../drm/amd/display/dc/core/dc_link_hwss.c| 3 + > .../gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c | 3 + > .../drm/amd/display/dc/dcn20/dcn20_hwseq.c| 4 - > .../gpu/drm/amd/display/dc/dcn20/dcn20_optc.c | 72 +--- > .../gpu/drm/amd/display/dc/dcn20/dcn20_optc.h | 3 - > .../drm/amd/display/dc/dcn20/dcn20_resource.c | 7 +- > .../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 + > .../display/dc/dcn20/dcn20_stream_encoder.c | 8 - > .../amd/display/dc/inc/hw/timing_generator.h | 2 - > drivers/gpu/drm/drm_dp_aux_dev.c | 12 +- > drivers/gpu/drm/drm_dp_helper.c | 10 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 240 +++ > include/drm/drm_dp_mst_helper.h | 8 +- > 18 files changed, 806 insertions(+), 131 deletions(-) > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 13/16] drm/amd/display: Validate DSC caps on MST endpoints
Whoops, left in a test print. Ignore this patch
From: David Francis
Sent: August 21, 2019 4:01 PM
To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org
Cc: Francis, David; Liu, Wenjing; Cornij, Nikola
Subject: [PATCH v3 13/16] drm/amd/display: Validate DSC caps on MST endpoints
During MST mode enumeration, if a new dc_sink is created,
populate it with dsc caps as appropriate.
Use drm_dp_mst_dsc_caps_for_port to get the raw caps,
then parse them onto dc_sink with dc_dsc_parse_dsc_dpcd.
Cc: Wenjing Liu
Cc: Nikola Cornij
Signed-off-by: David Francis
---
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 27 ++-
1 file changed, 26 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..9978c1a01eb7 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,24 @@ static const struct drm_connector_funcs
dm_dp_mst_connector_funcs = {
.early_unregister = amdgpu_dm_mst_connector_early_unregister,
};
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector
*aconnector)
+{
+ struct dc_sink *dc_sink = aconnector->dc_sink;
+ struct drm_dp_mst_port *port = aconnector->port;
+ u8 dsc_caps[16] = { 0 };
+
+ if (drm_dp_mst_dsc_caps_for_port(port, dsc_caps) < 0)
+ return false;
+
+ printk("Validated DSC caps 0x%x", dsc_caps[0]);
+ if (!dc_dsc_parse_dsc_dpcd(dsc_caps, NULL,
&dc_sink->sink_dsc_caps.dsc_dec_caps))
+ return false;
+
+ return true;
+}
+#endif
+
static int dm_dp_mst_get_modes(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
@@ -231,10 +250,16 @@ static int dm_dp_mst_get_modes(struct drm_connector
*connector)
/* dc_link_add_remote_sink returns a new reference */
aconnector->dc_sink = dc_sink;
- if (aconnector->dc_sink)
+ if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(
connector, aconnector->edid);
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ if (!validate_dsc_caps_on_connector(aconnector))
+ memset(&aconnector->dc_sink->sink_dsc_caps,
+ 0,
sizeof(aconnector->dc_sink->sink_dsc_caps));
+#endif
+ }
}
drm_connector_update_edid_property(
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 16/16] drm/amd/display: Trigger modesets on MST DSC connectors
I know this looks like it could be a DRM helper, but
- That would require a DRM-generic way of knowing if a
connector supports DSC, and current precedent is that
DSC functionality is stored on a driver-specific basis
- This function, by necessity, locks global state. Other
hardware may be able to solve this problem with more fine-
grained locking, for example by recalculating DSC configs
and then adding to the state all CRTCs for which the config
has changed
- AMD is currently the only user of MST DSC, so if this needs
to be in DRM, it would be easier to do so once it is
clear what functionality is common between drivers
From: dri-devel on behalf of David
Francis
Sent: August 21, 2019 4:01 PM
To: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org
Cc: Li, Sun peng (Leo); Francis, David; Kazlauskas, Nicholas
Subject: [PATCH v3 16/16] drm/amd/display: Trigger modesets on MST DSC
connectors
Whenever a connector on an MST network is attached, detached, or
undergoes a modeset, the DSC configs for each stream on that
topology will be recalculated. This can change their required
bandwidth, requiring a full reprogramming, as though a modeset
was performed, even if that stream did not change timing.
Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset,
for each crtc that shares a MST topology with that stream and
supports DSC, add that crtc (and all affected connectors and
planes) to the atomic state and set mode_changed on its state
v2: Do this check only on Navi and before adding connectors
and planes on modesetting crtcs
Cc: Leo Li
Cc: Nicholas Kazlauskas
Cc: Lyude Paul
Signed-off-by: David Francis
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 77 +++
1 file changed, 77 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 145fd73025dc..702fb0e29053 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6475,7 +6475,73 @@ static int do_aquire_global_lock(struct drm_device *dev,
return ret < 0 ? ret : 0;
}
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+/*
+ * TODO: This logic should at some point be moved into DRM
+ */
+static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct
drm_crtc *crtc)
+{
+ struct drm_connector *connector;
+ struct drm_connector_state *conn_state;
+ struct drm_connector_list_iter conn_iter;
+ struct drm_crtc_state *new_crtc_state;
+ struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;
+ int i, j;
+ struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 };
+
+ for_each_new_connector_in_state(state, connector, conn_state, i) {
+ if (conn_state->crtc != crtc)
+ continue;
+
+ aconnector = to_amdgpu_dm_connector(connector);
+ if (!aconnector->port)
+ aconnector = NULL;
+ else
+ break;
+ }
+
+ if (!aconnector)
+ return 0;
+
+ i = 0;
+ drm_connector_list_iter_begin(state->dev, &conn_iter);
+ drm_for_each_connector_iter(connector, &conn_iter) {
+ if (!connector->state || !connector->state->crtc)
+ continue;
+
+ aconnector_to_add = to_amdgpu_dm_connector(connector);
+ if (!aconnector_to_add->port)
+ continue;
+
+ if (aconnector_to_add->port->mgr != aconnector->port->mgr)
+ continue;
+
+ if (!aconnector_to_add->dc_sink)
+ continue;
+
+ if
(!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported)
+ continue;
+
+ if (i >= AMDGPU_MAX_CRTCS)
+ continue;
+
+ crtcs_affected[i] = connector->state->crtc;
+ i++;
+ }
+ drm_connector_list_iter_end(&conn_iter);
+
+ for (j = 0; j < i; j++) {
+ new_crtc_state = drm_atomic_get_crtc_state(state,
crtcs_affected[j]);
+ if (IS_ERR(new_crtc_state))
+ return PTR_ERR(new_crtc_state);
+ new_crtc_state->mode_changed = true;
+ }
+
+ return 0;
+
+}
+#endif
static void get_freesync_config_for_crtc(
struct dm_crtc_state *new_crtc_state,
struct dm_connector_state *new_con_state)
@@ -7160,6 +7226,17 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ if (adev->asic_type >= CHIP_NAVI10) {
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state,
Re: [PATCH 14/14] drm/amd/display: Trigger modesets on MST DSC connectors
ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +
>> +}
>> +#endif
>> static void get_freesync_config_for_crtc(
>>struct dm_crtc_state *new_crtc_state,
>>struct dm_connector_state *new_con_state)
>> @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device
>> *dev,
>>goto fail;
>>}
>>
>> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
>> new_crtc_state, i) {
>> + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
>> + ret = add_affected_mst_dsc_crtcs(state, crtc);
>> + if (ret)
>> + goto fail;
>> + }
>> + }
>> +#endif
>>/*
>> * Add all primary and overlay planes on the CRTC to the state
>> * whenever a plane is enabled to maintain correct z-ordering
From: Kazlauskas, Nicholas
Sent: August 19, 2019 3:34 PM
To: Francis, David; dri-devel@lists.freedesktop.org;
amd-...@lists.freedesktop.org
Cc: Li, Sun peng (Leo)
Subject: Re: [PATCH 14/14] drm/amd/display: Trigger modesets on MST DSC
connectors
On 8/19/19 11:50 AM, David Francis wrote:
> Whenever a connector on an MST network is attached, detached, or
> undergoes a modeset, the DSC configs for each stream on that
> topology will be recalculated. This can change their required
> bandwidth, requiring a full reprogramming, as though a modeset
> was performed, even if that stream did not change timing.
>
> Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset,
> for each crtc that shares a MST topology with that stream and
> supports DSC, add that crtc (and all affected connectors and
> planes) to the atomic state and set mode_changed on its state
>
> Cc: Leo Li
> Cc: Nicholas Kazlauskas
> Signed-off-by: David Francis
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 +++
> 1 file changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 145fd73025dc..8d5357aec5e8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6475,7 +6475,78 @@ static int do_aquire_global_lock(struct drm_device
> *dev,
>
> return ret < 0 ? ret : 0;
> }
> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct
> drm_crtc *crtc)
> +{
> + struct drm_connector *connector;
> + struct drm_connector_state *conn_state;
> + struct drm_connector_list_iter conn_iter;
> + struct drm_crtc_state *new_crtc_state;
> + struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;
> + int i, j, ret;
> + struct drm_crtc *crtcs_affected[MAX_PIPES] = { 0 }; > +
> + for_each_new_connector_in_state(state, connector, conn_state, i) {
> + if (conn_state->crtc != crtc)
> + continue;
> +
> + aconnector = to_amdgpu_dm_connector(connector);
> + if (!aconnector->port)
> + aconnector = NULL;
> + else
> + break;
> + }
> +
> + if (!aconnector)
> + return 0;
> +
> + i = 0;
> + drm_connector_list_iter_begin(state->dev, &conn_iter);
I don't like that we're grabbing the global connector lock every single
time any CRTC undergoes a modeset even for ASICs that don't support DSC.
We do lock everything below in atomic check anyway for FULL updates but
I'd like to avoid adding more code that does this if possible. Maybe a
check at the start that only does this if the ASIC has DSC support would
be OK.
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (!connector->state || !connector->state->crtc)
> + continue;
> +
> + aconnector_to_add = to_amdgpu_dm_connector(connector);
> + if (!aconnector_to_add->port)
> + continue;
> +
> + if (aconnector_to_add->port->mgr != aconnector->port->mgr)
> + continue;
> +
> + if (!aconnector_to_add->dc_sink)
> + continue;
> +
> + if
> (!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported)
>
Re: [PATCH 06/14] drm/amd/display: Use dc helpers to compute timeslot distribution
On 8/19/19 11:50 AM, David Francis wrote:
>> We were using drm helpers to convert a timing into its
>> bandwidth, its bandwidth into pbn, and its pbn into timeslots
>>
>> These helpers
>> -Did not take DSC timings into account
>> -Used the link rate and lane count of the link's aux device,
>> which are not the same as the link's current cap
>> -Did not take FEC into account (FEC reduces the PBN per timeslot)
>>
>> Use the DC helpers (dc_bandwidth_in_kbps_from_timing,
>> dc_link_bandwidth_kbps) instead
>>
>> Cc: Jerry Zuo
>> Signed-off-by: David Francis
>
>Wouldn't this be a good candidate for shared DRM helpers or to modify
>the existing ones to support this usecase?
>
>Seems like this would be shared across drivers.
>
>Nicholas Kazlauskas
>
These functions use information which is not stored in drm structs but tracked
in
a driver-specific way
- Whether or not FEC is enabled
- Whether or not DSC is enabled, and with what config
- The current link setting
This could be shifted into drm helpers, but it would require functions with
signatures like
drm_dp_calc_pbn_mode(clock, bpp, dsc_bpp)
and
drm_dp_find_vcpi_slots(mst_mgr, pbn, fec_enabled, lane_count, link_rate)
and each driver would need to convert their own structs into those fields
before calling these helpers.
Is that worth it?
>> ---
>> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 41 ---
>> 1 file changed, 8 insertions(+), 33 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 5f2c315b18ba..b32c0790399a 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
>> @@ -189,8 +189,8 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>>int slots = 0;
>>bool ret;
>>int clock;
>> - int bpp = 0;
>>int pbn = 0;
>> + int pbn_per_timeslot;
>>
>>aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
>>
>> @@ -205,40 +205,15 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>>mst_port = aconnector->port;
>>
>>if (enable) {
>> - clock = stream->timing.pix_clk_100hz / 10;
>> -
>> - switch (stream->timing.display_color_depth) {
>> -
>> - case COLOR_DEPTH_666:
>> - bpp = 6;
>> - break;
>> - case COLOR_DEPTH_888:
>> - bpp = 8;
>> - break;
>> - case COLOR_DEPTH_101010:
>> - bpp = 10;
>> - break;
>> - case COLOR_DEPTH_121212:
>> - bpp = 12;
>> - break;
>> - case COLOR_DEPTH_141414:
>> - bpp = 14;
>> - break;
>> - case COLOR_DEPTH_161616:
>> - bpp = 16;
>> - break;
>> - default:
>> - ASSERT(bpp != 0);
>> - break;
>> - }
>> -
>> - bpp = bpp * 3;
>> -
>> - /* TODO need to know link rate */
>> + clock = dc_bandwidth_in_kbps_from_timing(&stream->timing);
>>
>> - pbn = drm_dp_calc_pbn_mode(clock, bpp);
>> + /* dc_bandwidth_in_kbps_from_timing already takes bpp into
>> account */
>> + pbn = drm_dp_calc_pbn_mode(clock, 1);
>>
>> - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn);
>> + /* Convert kilobits per second / 64 (for 64 timeslots) to pbn
>> (54/64 megabytes per second) */
>> + pbn_per_timeslot = dc_link_bandwidth_kbps(
>> + stream->link,
>> dc_link_get_link_cap(stream->link)) / (8 * 1000 * 54);
>> + slots = DIV_ROUND_UP(pbn, pbn_per_timeslot);
>>ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots);
>>
>>if (!ret)
From: Kazlauskas, Nicholas
Sent: August 19, 2019 3:21 PM
To: Francis, David; dri-devel@lists.freedesktop.org;
amd-...@lists.freedesktop.org
Cc: Zuo, Jerry
Subject: Re: [PATCH 06/14] drm/amd/display: Use dc helpers to compute timeslot
distribution
On 8/19/19 11:50 AM, David Francis wrote:
> We were using drm helpers to convert a timing into its
> bandwidth, it
