2014-06-05 12:01:32 +08:00
|
|
|
/*
|
|
|
|
* Copyright © 2014 Red Hat.
|
|
|
|
*
|
|
|
|
* Permission to use, copy, modify, distribute, and sell this software and its
|
|
|
|
* documentation for any purpose is hereby granted without fee, provided that
|
|
|
|
* the above copyright notice appear in all copies and that both that copyright
|
|
|
|
* notice and this permission notice appear in supporting documentation, and
|
|
|
|
* that the name of the copyright holders not be used in advertising or
|
|
|
|
* publicity pertaining to distribution of the software without specific,
|
|
|
|
* written prior permission. The copyright holders make no representations
|
|
|
|
* about the suitability of this software for any purpose. It is provided "as
|
|
|
|
* is" without express or implied warranty.
|
|
|
|
*
|
|
|
|
* THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
|
|
|
|
* INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
|
|
|
|
* EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
|
|
|
|
* CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
|
|
|
|
* DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
|
|
|
|
* TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
|
|
|
|
* OF THIS SOFTWARE.
|
|
|
|
*/
|
|
|
|
#ifndef _DRM_DP_MST_HELPER_H_
|
|
|
|
#define _DRM_DP_MST_HELPER_H_
|
|
|
|
|
|
|
|
#include <linux/types.h>
|
|
|
|
#include <drm/drm_dp_helper.h>
|
2017-04-21 13:51:31 +08:00
|
|
|
#include <drm/drm_atomic.h>
|
2014-06-05 12:01:32 +08:00
|
|
|
|
|
|
|
struct drm_dp_mst_branch;
|
|
|
|
|
|
|
|
/**
|
2014-10-20 22:53:13 +08:00
|
|
|
* struct drm_dp_vcpi - Virtual Channel Payload Identifier
|
2014-06-05 12:01:32 +08:00
|
|
|
* @vcpi: Virtual channel ID.
|
|
|
|
* @pbn: Payload Bandwidth Number for this channel
|
|
|
|
* @aligned_pbn: PBN aligned with slot size
|
|
|
|
* @num_slots: number of slots for this PBN
|
|
|
|
*/
|
|
|
|
struct drm_dp_vcpi {
|
|
|
|
int vcpi;
|
|
|
|
int pbn;
|
|
|
|
int aligned_pbn;
|
|
|
|
int num_slots;
|
|
|
|
};
|
|
|
|
|
|
|
|
/**
|
|
|
|
* struct drm_dp_mst_port - MST port
|
|
|
|
* @port_num: port number
|
drm/dp_mst: Protect drm_dp_mst_port members with locking
This is a complicated one. Essentially, there's currently a problem in the MST
core that hasn't really caused any issues that we're aware of (emphasis on "that
we're aware of"): locking.
When we go through and probe the link addresses and path resources in a
topology, we hold no locks when updating ports with said information. The
members I'm referring to in particular are:
- ldps
- ddps
- mcs
- pdt
- dpcd_rev
- num_sdp_streams
- num_sdp_stream_sinks
- available_pbn
- input
- connector
Now that we're handling UP requests asynchronously and will be using some of
the struct members mentioned above in atomic modesetting in the future for
features such as PBN validation, this is going to become a lot more important.
As well, the next few commits that prepare us for and introduce suspend/resume
reprobing will also need clear locking in order to prevent from additional
racing hilarities that we never could have hit in the past.
So, let's solve this issue by using &mgr->base.lock, the modesetting
lock which currently only protects &mgr->base.state. This works
perfectly because it allows us to avoid blocking connection_mutex
unnecessarily, and we can grab this in connector detection paths since
it's a ww mutex. We start by having drm_dp_mst_handle_up_req() hold this
when updating ports. For drm_dp_mst_handle_link_address_port() things
are a bit more complicated. As I've learned the hard way, we can grab
&mgr->lock.base for everything except for port->connector. See, our
normal driver probing paths end up generating this rather obvious
lockdep chain:
&drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
However, sysfs grabs &drm->mode_config.mutex in order to protect itself
from connector state changing under it. Because this entails grabbing
kn->count, e.g. the lock that the kernel provides for protecting sysfs
contexts, we end up grabbing kn->count followed by
&drm->mode_config.mutex. This ends up creating an extremely rude chain:
&kn->count
-> &drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
I mean, look at that thing! It's just evil!!! This gross thing ends up
making any calls to drm_connector_register()/drm_connector_unregister()
impossible when holding any kind of modesetting lock. This is annoying
because ideally, we always want to ensure that
drm_dp_mst_port->connector never changes when doing an atomic commit or
check that would affect the atomic topology state so that it can
reliably and easily be used from future DRM DP MST helpers to assist
with tasks such as scanning through the current VCPI allocations and
adding connectors which need to have their allocations updated in
response to a bandwidth change or the like.
Being able to hold &mgr->base.lock throughout the entire link probe
process would have been _great_, since we could prevent userspace from
ever seeing any states in-between individual port changes and as a
result likely end up with a much faster probe and more consistent
results from said probes. But without some rework of how we handle
connector probing in sysfs it's not at all currently possible. In the
future, maybe we can try using the sysfs locks to protect updates to
connector probing state and fix this mess.
So for now, to protect everything other than port->connector under
&mgr->base.lock and ensure that we still have the guarantee that atomic
check/commit contexts will never see port->connector change we use a
silly trick. See: port->connector only needs to change in order to
ensure that input ports (see the MST spec) never have a ghost connector
associated with them. But, there's nothing stopping us from simply
throwing the entire port out and creating a new one in order to maintain
that requirement while still keeping port->connector consistent across
the lifetime of the port in atomic check/commit contexts. For all
intended purposes this works fine, as we validate ports in any contexts
we care about before using them and as such will end up reporting the
connector as disconnected until it's port's destruction finalizes. So,
we just do that in cases where we detect port->input has transitioned
from true->false. We don't need to worry about the other direction,
since a port without a connector isn't visible to userspace and as such
doesn't need to be protected by &mgr->base.lock until we finish
registering a connector for it.
For updating members of drm_dp_mst_port other than port->connector, we
simply grab &mgr->base.lock in drm_dp_mst_link_probe_work() for already
registered ports, update said members and drop the lock before
potentially registering a connector and probing the link address of it's
children.
Finally, we modify drm_dp_mst_detect_port() to take a modesetting lock
acquisition context in order to acquire &mgr->base.lock under
&connection_mutex and convert all it's users over to using the
.detect_ctx probe hooks.
With that, we finally have well defined locking.
Changes since v4:
* Get rid of port->mutex, stop using connection_mutex and just use our own
modesetting lock - mgr->base.lock. Also, add a probe_lock that comes
before this patch.
* Just throw out ports that get changed from an output to an input, and
replace them with new ports. This lets us ensure that modesetting
contexts never see port->connector go from having a connector to being
NULL.
* Write an extremely detailed explanation of what problems this is
trying to fix, since there's a _lot_ of context here and I honestly
forgot some of it myself a couple times.
* Don't grab mgr->lock when reading port->mstb in
drm_dp_mst_handle_link_address_port(). It's not needed.
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-7-lyude@redhat.com
2019-06-18 05:59:29 +08:00
|
|
|
* @input: if this port is an input port. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @mcs: message capability status - DP 1.2 spec. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @ddps: DisplayPort Device Plug Status - DP 1.2. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @pdt: Peer Device Type. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @ldps: Legacy Device Plug Status. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @dpcd_rev: DPCD revision of device on this port. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @num_sdp_streams: Number of simultaneous streams. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @num_sdp_stream_sinks: Number of stream sinks. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
|
|
|
* @available_pbn: Available bandwidth for this port. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
2014-06-05 12:01:32 +08:00
|
|
|
* @next: link to next port on this branch device
|
2019-03-14 01:51:41 +08:00
|
|
|
* @aux: i2c aux transport to talk to device connected to this port, protected
|
drm/dp_mst: Protect drm_dp_mst_port members with locking
This is a complicated one. Essentially, there's currently a problem in the MST
core that hasn't really caused any issues that we're aware of (emphasis on "that
we're aware of"): locking.
When we go through and probe the link addresses and path resources in a
topology, we hold no locks when updating ports with said information. The
members I'm referring to in particular are:
- ldps
- ddps
- mcs
- pdt
- dpcd_rev
- num_sdp_streams
- num_sdp_stream_sinks
- available_pbn
- input
- connector
Now that we're handling UP requests asynchronously and will be using some of
the struct members mentioned above in atomic modesetting in the future for
features such as PBN validation, this is going to become a lot more important.
As well, the next few commits that prepare us for and introduce suspend/resume
reprobing will also need clear locking in order to prevent from additional
racing hilarities that we never could have hit in the past.
So, let's solve this issue by using &mgr->base.lock, the modesetting
lock which currently only protects &mgr->base.state. This works
perfectly because it allows us to avoid blocking connection_mutex
unnecessarily, and we can grab this in connector detection paths since
it's a ww mutex. We start by having drm_dp_mst_handle_up_req() hold this
when updating ports. For drm_dp_mst_handle_link_address_port() things
are a bit more complicated. As I've learned the hard way, we can grab
&mgr->lock.base for everything except for port->connector. See, our
normal driver probing paths end up generating this rather obvious
lockdep chain:
&drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
However, sysfs grabs &drm->mode_config.mutex in order to protect itself
from connector state changing under it. Because this entails grabbing
kn->count, e.g. the lock that the kernel provides for protecting sysfs
contexts, we end up grabbing kn->count followed by
&drm->mode_config.mutex. This ends up creating an extremely rude chain:
&kn->count
-> &drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
I mean, look at that thing! It's just evil!!! This gross thing ends up
making any calls to drm_connector_register()/drm_connector_unregister()
impossible when holding any kind of modesetting lock. This is annoying
because ideally, we always want to ensure that
drm_dp_mst_port->connector never changes when doing an atomic commit or
check that would affect the atomic topology state so that it can
reliably and easily be used from future DRM DP MST helpers to assist
with tasks such as scanning through the current VCPI allocations and
adding connectors which need to have their allocations updated in
response to a bandwidth change or the like.
Being able to hold &mgr->base.lock throughout the entire link probe
process would have been _great_, since we could prevent userspace from
ever seeing any states in-between individual port changes and as a
result likely end up with a much faster probe and more consistent
results from said probes. But without some rework of how we handle
connector probing in sysfs it's not at all currently possible. In the
future, maybe we can try using the sysfs locks to protect updates to
connector probing state and fix this mess.
So for now, to protect everything other than port->connector under
&mgr->base.lock and ensure that we still have the guarantee that atomic
check/commit contexts will never see port->connector change we use a
silly trick. See: port->connector only needs to change in order to
ensure that input ports (see the MST spec) never have a ghost connector
associated with them. But, there's nothing stopping us from simply
throwing the entire port out and creating a new one in order to maintain
that requirement while still keeping port->connector consistent across
the lifetime of the port in atomic check/commit contexts. For all
intended purposes this works fine, as we validate ports in any contexts
we care about before using them and as such will end up reporting the
connector as disconnected until it's port's destruction finalizes. So,
we just do that in cases where we detect port->input has transitioned
from true->false. We don't need to worry about the other direction,
since a port without a connector isn't visible to userspace and as such
doesn't need to be protected by &mgr->base.lock until we finish
registering a connector for it.
For updating members of drm_dp_mst_port other than port->connector, we
simply grab &mgr->base.lock in drm_dp_mst_link_probe_work() for already
registered ports, update said members and drop the lock before
potentially registering a connector and probing the link address of it's
children.
Finally, we modify drm_dp_mst_detect_port() to take a modesetting lock
acquisition context in order to acquire &mgr->base.lock under
&connection_mutex and convert all it's users over to using the
.detect_ctx probe hooks.
With that, we finally have well defined locking.
Changes since v4:
* Get rid of port->mutex, stop using connection_mutex and just use our own
modesetting lock - mgr->base.lock. Also, add a probe_lock that comes
before this patch.
* Just throw out ports that get changed from an output to an input, and
replace them with new ports. This lets us ensure that modesetting
contexts never see port->connector go from having a connector to being
NULL.
* Write an extremely detailed explanation of what problems this is
trying to fix, since there's a _lot_ of context here and I honestly
forgot some of it myself a couple times.
* Don't grab mgr->lock when reading port->mstb in
drm_dp_mst_handle_link_address_port(). It's not needed.
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-7-lyude@redhat.com
2019-06-18 05:59:29 +08:00
|
|
|
* by &drm_dp_mst_topology_mgr.base.lock.
|
2014-06-05 12:01:32 +08:00
|
|
|
* @parent: branch device parent of this port
|
|
|
|
* @vcpi: Virtual Channel Payload info for this port.
|
drm/dp_mst: Protect drm_dp_mst_port members with locking
This is a complicated one. Essentially, there's currently a problem in the MST
core that hasn't really caused any issues that we're aware of (emphasis on "that
we're aware of"): locking.
When we go through and probe the link addresses and path resources in a
topology, we hold no locks when updating ports with said information. The
members I'm referring to in particular are:
- ldps
- ddps
- mcs
- pdt
- dpcd_rev
- num_sdp_streams
- num_sdp_stream_sinks
- available_pbn
- input
- connector
Now that we're handling UP requests asynchronously and will be using some of
the struct members mentioned above in atomic modesetting in the future for
features such as PBN validation, this is going to become a lot more important.
As well, the next few commits that prepare us for and introduce suspend/resume
reprobing will also need clear locking in order to prevent from additional
racing hilarities that we never could have hit in the past.
So, let's solve this issue by using &mgr->base.lock, the modesetting
lock which currently only protects &mgr->base.state. This works
perfectly because it allows us to avoid blocking connection_mutex
unnecessarily, and we can grab this in connector detection paths since
it's a ww mutex. We start by having drm_dp_mst_handle_up_req() hold this
when updating ports. For drm_dp_mst_handle_link_address_port() things
are a bit more complicated. As I've learned the hard way, we can grab
&mgr->lock.base for everything except for port->connector. See, our
normal driver probing paths end up generating this rather obvious
lockdep chain:
&drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
However, sysfs grabs &drm->mode_config.mutex in order to protect itself
from connector state changing under it. Because this entails grabbing
kn->count, e.g. the lock that the kernel provides for protecting sysfs
contexts, we end up grabbing kn->count followed by
&drm->mode_config.mutex. This ends up creating an extremely rude chain:
&kn->count
-> &drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
I mean, look at that thing! It's just evil!!! This gross thing ends up
making any calls to drm_connector_register()/drm_connector_unregister()
impossible when holding any kind of modesetting lock. This is annoying
because ideally, we always want to ensure that
drm_dp_mst_port->connector never changes when doing an atomic commit or
check that would affect the atomic topology state so that it can
reliably and easily be used from future DRM DP MST helpers to assist
with tasks such as scanning through the current VCPI allocations and
adding connectors which need to have their allocations updated in
response to a bandwidth change or the like.
Being able to hold &mgr->base.lock throughout the entire link probe
process would have been _great_, since we could prevent userspace from
ever seeing any states in-between individual port changes and as a
result likely end up with a much faster probe and more consistent
results from said probes. But without some rework of how we handle
connector probing in sysfs it's not at all currently possible. In the
future, maybe we can try using the sysfs locks to protect updates to
connector probing state and fix this mess.
So for now, to protect everything other than port->connector under
&mgr->base.lock and ensure that we still have the guarantee that atomic
check/commit contexts will never see port->connector change we use a
silly trick. See: port->connector only needs to change in order to
ensure that input ports (see the MST spec) never have a ghost connector
associated with them. But, there's nothing stopping us from simply
throwing the entire port out and creating a new one in order to maintain
that requirement while still keeping port->connector consistent across
the lifetime of the port in atomic check/commit contexts. For all
intended purposes this works fine, as we validate ports in any contexts
we care about before using them and as such will end up reporting the
connector as disconnected until it's port's destruction finalizes. So,
we just do that in cases where we detect port->input has transitioned
from true->false. We don't need to worry about the other direction,
since a port without a connector isn't visible to userspace and as such
doesn't need to be protected by &mgr->base.lock until we finish
registering a connector for it.
For updating members of drm_dp_mst_port other than port->connector, we
simply grab &mgr->base.lock in drm_dp_mst_link_probe_work() for already
registered ports, update said members and drop the lock before
potentially registering a connector and probing the link address of it's
children.
Finally, we modify drm_dp_mst_detect_port() to take a modesetting lock
acquisition context in order to acquire &mgr->base.lock under
&connection_mutex and convert all it's users over to using the
.detect_ctx probe hooks.
With that, we finally have well defined locking.
Changes since v4:
* Get rid of port->mutex, stop using connection_mutex and just use our own
modesetting lock - mgr->base.lock. Also, add a probe_lock that comes
before this patch.
* Just throw out ports that get changed from an output to an input, and
replace them with new ports. This lets us ensure that modesetting
contexts never see port->connector go from having a connector to being
NULL.
* Write an extremely detailed explanation of what problems this is
trying to fix, since there's a _lot_ of context here and I honestly
forgot some of it myself a couple times.
* Don't grab mgr->lock when reading port->mstb in
drm_dp_mst_handle_link_address_port(). It's not needed.
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-7-lyude@redhat.com
2019-06-18 05:59:29 +08:00
|
|
|
* @connector: DRM connector this port is connected to. Protected by
|
|
|
|
* &drm_dp_mst_topology_mgr.base.lock.
|
2014-06-05 12:01:32 +08:00
|
|
|
* @mgr: topology manager this port lives under.
|
|
|
|
*
|
|
|
|
* This structure represents an MST port endpoint on a device somewhere
|
|
|
|
* in the MST topology.
|
|
|
|
*/
|
|
|
|
struct drm_dp_mst_port {
|
drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.
To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:
commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;
commit 263efde31f97 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.
Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:
commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref")
But then that introduced use-after-free errors, so I quickly reverted
it:
commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"")
And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortunately, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.
After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.
To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.
Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.
Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:
- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)
Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.
Changes since v4:
* Change the kernel-figure summary for dp-mst/topology-figure-1.dot a
bit - danvet
* Remove figure numbers - danvet
Changes since v3:
* Remove rebase detritus - danvet
* Split out purely style changes into separate patches - hwentlan
Changes since v2:
* Fix commit message - checkpatch
* s/)-1/) - 1/g - checkpatch
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
and payloads - danvet
* Make suggested documentation changes for
drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() -
danvet
* Better explain the relationship between malloc refs and topology krefs
in the documentation for drm_dp_mst_topology_get_port() and
drm_dp_mst_topology_get_mstb() - danvet
* Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet
* Rename drm_dp_mst_topology_get_(port|mstb)() ->
drm_dp_mst_topology_try_get_(port|mstb)() and
drm_dp_mst_topology_ref_(port|mstb)() ->
drm_dp_mst_topology_get_(port|mstb)() - danvet
* s/should/must in docs - danvet
* WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet
* Move kdocs for mstb/port structs inline - danvet
* Split drm_dp_get_last_connected_port_and_mstb() changes into their own
commit - danvet
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com
2019-01-11 08:53:29 +08:00
|
|
|
/**
|
|
|
|
* @topology_kref: refcount for this port's lifetime in the topology,
|
|
|
|
* only the DP MST helpers should need to touch this
|
|
|
|
*/
|
|
|
|
struct kref topology_kref;
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @malloc_kref: refcount for the memory allocation containing this
|
|
|
|
* structure. See drm_dp_mst_get_port_malloc() and
|
|
|
|
* drm_dp_mst_put_port_malloc().
|
|
|
|
*/
|
|
|
|
struct kref malloc_kref;
|
2014-06-05 12:01:32 +08:00
|
|
|
|
|
|
|
u8 port_num;
|
|
|
|
bool input;
|
|
|
|
bool mcs;
|
|
|
|
bool ddps;
|
|
|
|
u8 pdt;
|
|
|
|
bool ldps;
|
|
|
|
u8 dpcd_rev;
|
|
|
|
u8 num_sdp_streams;
|
|
|
|
u8 num_sdp_stream_sinks;
|
|
|
|
uint16_t available_pbn;
|
|
|
|
struct list_head next;
|
2019-10-17 04:02:59 +08:00
|
|
|
/**
|
|
|
|
* @mstb: the branch device connected to this port, if there is one.
|
|
|
|
* This should be considered protected for reading by
|
|
|
|
* &drm_dp_mst_topology_mgr.lock. There are two exceptions to this:
|
|
|
|
* &drm_dp_mst_topology_mgr.up_req_work and
|
|
|
|
* &drm_dp_mst_topology_mgr.work, which do not grab
|
|
|
|
* &drm_dp_mst_topology_mgr.lock during reads but are the only
|
|
|
|
* updaters of this list and are protected from writing concurrently
|
|
|
|
* by &drm_dp_mst_topology_mgr.probe_lock.
|
|
|
|
*/
|
|
|
|
struct drm_dp_mst_branch *mstb;
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_aux aux; /* i2c bus for this port? */
|
|
|
|
struct drm_dp_mst_branch *parent;
|
|
|
|
|
|
|
|
struct drm_dp_vcpi vcpi;
|
|
|
|
struct drm_connector *connector;
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr;
|
2014-10-20 14:28:02 +08:00
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @cached_edid: for DP logical ports - make tiling work by ensuring
|
|
|
|
* that the EDID for all connectors is read immediately.
|
|
|
|
*/
|
|
|
|
struct edid *cached_edid;
|
|
|
|
/**
|
|
|
|
* @has_audio: Tracks whether the sink connector to this port is
|
|
|
|
* audio-capable.
|
|
|
|
*/
|
2015-12-02 14:09:43 +08:00
|
|
|
bool has_audio;
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
|
|
|
/**
|
|
|
|
* struct drm_dp_mst_branch - MST branch device.
|
|
|
|
* @rad: Relative Address to talk to this branch device.
|
|
|
|
* @lct: Link count total to talk to this branch device.
|
|
|
|
* @num_ports: number of ports on the branch.
|
|
|
|
* @msg_slots: one bit per transmitted msg slot.
|
|
|
|
* @port_parent: pointer to the port parent, NULL if toplevel.
|
|
|
|
* @mgr: topology manager for this branch device.
|
|
|
|
* @tx_slots: transmission slots for this device.
|
|
|
|
* @last_seqno: last sequence number used to talk to this.
|
|
|
|
* @link_address_sent: if a link address message has been sent to this device yet.
|
2016-01-23 06:07:28 +08:00
|
|
|
* @guid: guid for DP 1.2 branch device. port under this branch can be
|
|
|
|
* identified by port #.
|
2014-06-05 12:01:32 +08:00
|
|
|
*
|
|
|
|
* This structure represents an MST branch device, there is one
|
2016-01-23 06:07:28 +08:00
|
|
|
* primary branch device at the root, along with any other branches connected
|
|
|
|
* to downstream port of parent branches.
|
2014-06-05 12:01:32 +08:00
|
|
|
*/
|
|
|
|
struct drm_dp_mst_branch {
|
drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.
To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:
commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;
commit 263efde31f97 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.
Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:
commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref")
But then that introduced use-after-free errors, so I quickly reverted
it:
commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"")
And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortunately, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.
After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.
To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.
Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.
Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:
- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)
Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.
Changes since v4:
* Change the kernel-figure summary for dp-mst/topology-figure-1.dot a
bit - danvet
* Remove figure numbers - danvet
Changes since v3:
* Remove rebase detritus - danvet
* Split out purely style changes into separate patches - hwentlan
Changes since v2:
* Fix commit message - checkpatch
* s/)-1/) - 1/g - checkpatch
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
and payloads - danvet
* Make suggested documentation changes for
drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() -
danvet
* Better explain the relationship between malloc refs and topology krefs
in the documentation for drm_dp_mst_topology_get_port() and
drm_dp_mst_topology_get_mstb() - danvet
* Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet
* Rename drm_dp_mst_topology_get_(port|mstb)() ->
drm_dp_mst_topology_try_get_(port|mstb)() and
drm_dp_mst_topology_ref_(port|mstb)() ->
drm_dp_mst_topology_get_(port|mstb)() - danvet
* s/should/must in docs - danvet
* WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet
* Move kdocs for mstb/port structs inline - danvet
* Split drm_dp_get_last_connected_port_and_mstb() changes into their own
commit - danvet
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com
2019-01-11 08:53:29 +08:00
|
|
|
/**
|
|
|
|
* @topology_kref: refcount for this branch device's lifetime in the
|
|
|
|
* topology, only the DP MST helpers should need to touch this
|
|
|
|
*/
|
|
|
|
struct kref topology_kref;
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @malloc_kref: refcount for the memory allocation containing this
|
|
|
|
* structure. See drm_dp_mst_get_mstb_malloc() and
|
|
|
|
* drm_dp_mst_put_mstb_malloc().
|
|
|
|
*/
|
|
|
|
struct kref malloc_kref;
|
|
|
|
|
2019-02-20 06:41:02 +08:00
|
|
|
/**
|
|
|
|
* @destroy_next: linked-list entry used by
|
|
|
|
* drm_dp_delayed_destroy_work()
|
|
|
|
*/
|
|
|
|
struct list_head destroy_next;
|
|
|
|
|
2014-06-05 12:01:32 +08:00
|
|
|
u8 rad[8];
|
|
|
|
u8 lct;
|
|
|
|
int num_ports;
|
|
|
|
|
|
|
|
int msg_slots;
|
2019-10-17 04:02:59 +08:00
|
|
|
/**
|
|
|
|
* @ports: the list of ports on this branch device. This should be
|
|
|
|
* considered protected for reading by &drm_dp_mst_topology_mgr.lock.
|
|
|
|
* There are two exceptions to this:
|
|
|
|
* &drm_dp_mst_topology_mgr.up_req_work and
|
|
|
|
* &drm_dp_mst_topology_mgr.work, which do not grab
|
|
|
|
* &drm_dp_mst_topology_mgr.lock during reads but are the only
|
|
|
|
* updaters of this list and are protected from updating the list
|
|
|
|
* concurrently by @drm_dp_mst_topology_mgr.probe_lock
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct list_head ports;
|
|
|
|
|
|
|
|
/* list of tx ops queue for this port */
|
|
|
|
struct drm_dp_mst_port *port_parent;
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr;
|
|
|
|
|
|
|
|
/* slots are protected by mstb->mgr->qlock */
|
|
|
|
struct drm_dp_sideband_msg_tx *tx_slots[2];
|
|
|
|
int last_seqno;
|
|
|
|
bool link_address_sent;
|
2016-01-23 06:07:28 +08:00
|
|
|
|
|
|
|
/* global unique identifier to identify branch devices */
|
|
|
|
u8 guid[16];
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
|
|
|
|
|
|
|
/* sideband msg header - not bit struct */
|
|
|
|
struct drm_dp_sideband_msg_hdr {
|
|
|
|
u8 lct;
|
|
|
|
u8 lcr;
|
|
|
|
u8 rad[8];
|
|
|
|
bool broadcast;
|
|
|
|
bool path_msg;
|
|
|
|
u8 msg_len;
|
|
|
|
bool somt;
|
|
|
|
bool eomt;
|
|
|
|
bool seqno;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_nak_reply {
|
|
|
|
u8 guid[16];
|
|
|
|
u8 reason;
|
|
|
|
u8 nak_data;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_link_address_ack_reply {
|
|
|
|
u8 guid[16];
|
|
|
|
u8 nports;
|
|
|
|
struct drm_dp_link_addr_reply_port {
|
|
|
|
bool input_port;
|
|
|
|
u8 peer_device_type;
|
|
|
|
u8 port_number;
|
|
|
|
bool mcs;
|
|
|
|
bool ddps;
|
|
|
|
bool legacy_device_plug_status;
|
|
|
|
u8 dpcd_revision;
|
|
|
|
u8 peer_guid[16];
|
|
|
|
u8 num_sdp_streams;
|
|
|
|
u8 num_sdp_stream_sinks;
|
|
|
|
} ports[16];
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_read_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u8 num_bytes;
|
|
|
|
u8 bytes[255];
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_write_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_write_nak_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u8 reason;
|
|
|
|
u8 bytes_written_before_failure;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_read_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u8 num_bytes;
|
|
|
|
u8 bytes[255];
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_read_nak_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u8 nak_reason;
|
|
|
|
u8 i2c_nak_transaction;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_write_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
|
|
struct drm_dp_sideband_msg_rx {
|
|
|
|
u8 chunk[48];
|
|
|
|
u8 msg[256];
|
|
|
|
u8 curchunk_len;
|
|
|
|
u8 curchunk_idx; /* chunk we are parsing now */
|
|
|
|
u8 curchunk_hdrlen;
|
|
|
|
u8 curlen; /* total length of the msg */
|
|
|
|
bool have_somt;
|
|
|
|
bool have_eomt;
|
|
|
|
struct drm_dp_sideband_msg_hdr initial_hdr;
|
|
|
|
};
|
|
|
|
|
2015-12-02 14:09:43 +08:00
|
|
|
#define DRM_DP_MAX_SDP_STREAMS 16
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_allocate_payload {
|
|
|
|
u8 port_number;
|
|
|
|
u8 number_sdp_streams;
|
|
|
|
u8 vcpi;
|
|
|
|
u16 pbn;
|
2015-12-02 14:09:43 +08:00
|
|
|
u8 sdp_stream_sink[DRM_DP_MAX_SDP_STREAMS];
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_allocate_payload_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u8 vcpi;
|
|
|
|
u16 allocated_pbn;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_connection_status_notify {
|
|
|
|
u8 guid[16];
|
|
|
|
u8 port_number;
|
|
|
|
bool legacy_device_plug_status;
|
|
|
|
bool displayport_device_plug_status;
|
|
|
|
bool message_capability_status;
|
|
|
|
bool input_port;
|
|
|
|
u8 peer_device_type;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_read {
|
|
|
|
u8 port_number;
|
|
|
|
u32 dpcd_address;
|
|
|
|
u8 num_bytes;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_write {
|
|
|
|
u8 port_number;
|
|
|
|
u32 dpcd_address;
|
|
|
|
u8 num_bytes;
|
|
|
|
u8 *bytes;
|
|
|
|
};
|
|
|
|
|
2015-10-14 16:51:17 +08:00
|
|
|
#define DP_REMOTE_I2C_READ_MAX_TRANSACTIONS 4
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_remote_i2c_read {
|
|
|
|
u8 num_transactions;
|
|
|
|
u8 port_number;
|
2019-09-04 04:45:45 +08:00
|
|
|
struct drm_dp_remote_i2c_read_tx {
|
2014-06-05 12:01:32 +08:00
|
|
|
u8 i2c_dev_id;
|
|
|
|
u8 num_bytes;
|
|
|
|
u8 *bytes;
|
|
|
|
u8 no_stop_bit;
|
|
|
|
u8 i2c_transaction_delay;
|
2015-10-14 16:51:17 +08:00
|
|
|
} transactions[DP_REMOTE_I2C_READ_MAX_TRANSACTIONS];
|
2014-06-05 12:01:32 +08:00
|
|
|
u8 read_i2c_device_id;
|
|
|
|
u8 num_bytes_read;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_write {
|
|
|
|
u8 port_number;
|
|
|
|
u8 write_i2c_device_id;
|
|
|
|
u8 num_bytes;
|
|
|
|
u8 *bytes;
|
|
|
|
};
|
|
|
|
|
|
|
|
/* this covers ENUM_RESOURCES, POWER_DOWN_PHY, POWER_UP_PHY */
|
|
|
|
struct drm_dp_port_number_req {
|
|
|
|
u8 port_number;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_enum_path_resources_ack_reply {
|
|
|
|
u8 port_number;
|
|
|
|
u16 full_payload_bw_number;
|
|
|
|
u16 avail_payload_bw_number;
|
|
|
|
};
|
|
|
|
|
|
|
|
/* covers POWER_DOWN_PHY, POWER_UP_PHY */
|
|
|
|
struct drm_dp_port_number_rep {
|
|
|
|
u8 port_number;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_query_payload {
|
|
|
|
u8 port_number;
|
|
|
|
u8 vcpi;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_resource_status_notify {
|
|
|
|
u8 port_number;
|
|
|
|
u8 guid[16];
|
|
|
|
u16 available_pbn;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_query_payload_ack_reply {
|
|
|
|
u8 port_number;
|
2019-08-30 00:52:19 +08:00
|
|
|
u16 allocated_pbn;
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_sideband_msg_req_body {
|
|
|
|
u8 req_type;
|
|
|
|
union ack_req {
|
|
|
|
struct drm_dp_connection_status_notify conn_stat;
|
|
|
|
struct drm_dp_port_number_req port_num;
|
|
|
|
struct drm_dp_resource_status_notify resource_stat;
|
|
|
|
|
|
|
|
struct drm_dp_query_payload query_payload;
|
|
|
|
struct drm_dp_allocate_payload allocate_payload;
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_read dpcd_read;
|
|
|
|
struct drm_dp_remote_dpcd_write dpcd_write;
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_read i2c_read;
|
|
|
|
struct drm_dp_remote_i2c_write i2c_write;
|
|
|
|
} u;
|
|
|
|
};
|
|
|
|
|
|
|
|
struct drm_dp_sideband_msg_reply_body {
|
|
|
|
u8 reply_type;
|
|
|
|
u8 req_type;
|
|
|
|
union ack_replies {
|
|
|
|
struct drm_dp_nak_reply nak;
|
|
|
|
struct drm_dp_link_address_ack_reply link_addr;
|
|
|
|
struct drm_dp_port_number_rep port_number;
|
|
|
|
|
|
|
|
struct drm_dp_enum_path_resources_ack_reply path_resources;
|
|
|
|
struct drm_dp_allocate_payload_ack_reply allocate_payload;
|
|
|
|
struct drm_dp_query_payload_ack_reply query_payload;
|
|
|
|
|
|
|
|
struct drm_dp_remote_dpcd_read_ack_reply remote_dpcd_read_ack;
|
|
|
|
struct drm_dp_remote_dpcd_write_ack_reply remote_dpcd_write_ack;
|
|
|
|
struct drm_dp_remote_dpcd_write_nak_reply remote_dpcd_write_nack;
|
|
|
|
|
|
|
|
struct drm_dp_remote_i2c_read_ack_reply remote_i2c_read_ack;
|
|
|
|
struct drm_dp_remote_i2c_read_nak_reply remote_i2c_read_nack;
|
|
|
|
struct drm_dp_remote_i2c_write_ack_reply remote_i2c_write_ack;
|
|
|
|
} u;
|
|
|
|
};
|
|
|
|
|
|
|
|
/* msg is queued to be put into a slot */
|
|
|
|
#define DRM_DP_SIDEBAND_TX_QUEUED 0
|
|
|
|
/* msg has started transmitting on a slot - still on msgq */
|
|
|
|
#define DRM_DP_SIDEBAND_TX_START_SEND 1
|
|
|
|
/* msg has finished transmitting on a slot - removed from msgq only in slot */
|
|
|
|
#define DRM_DP_SIDEBAND_TX_SENT 2
|
|
|
|
/* msg has received a response - removed from slot */
|
|
|
|
#define DRM_DP_SIDEBAND_TX_RX 3
|
|
|
|
#define DRM_DP_SIDEBAND_TX_TIMEOUT 4
|
|
|
|
|
|
|
|
struct drm_dp_sideband_msg_tx {
|
|
|
|
u8 msg[256];
|
|
|
|
u8 chunk[48];
|
|
|
|
u8 cur_offset;
|
|
|
|
u8 cur_len;
|
|
|
|
struct drm_dp_mst_branch *dst;
|
|
|
|
struct list_head next;
|
|
|
|
int seqno;
|
|
|
|
int state;
|
|
|
|
bool path_msg;
|
|
|
|
struct drm_dp_sideband_msg_reply_body reply;
|
|
|
|
};
|
|
|
|
|
|
|
|
/* sideband msg handler */
|
|
|
|
struct drm_dp_mst_topology_mgr;
|
|
|
|
struct drm_dp_mst_topology_cbs {
|
|
|
|
/* create a connector for a port */
|
2014-05-13 17:38:36 +08:00
|
|
|
struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
|
2015-09-16 15:55:23 +08:00
|
|
|
void (*register_connector)(struct drm_connector *connector);
|
2014-06-05 12:01:32 +08:00
|
|
|
void (*destroy_connector)(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_connector *connector);
|
|
|
|
};
|
|
|
|
|
|
|
|
#define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
|
|
|
|
|
|
|
|
#define DP_PAYLOAD_LOCAL 1
|
|
|
|
#define DP_PAYLOAD_REMOTE 2
|
|
|
|
#define DP_PAYLOAD_DELETE_LOCAL 3
|
|
|
|
|
|
|
|
struct drm_dp_payload {
|
|
|
|
int payload_state;
|
|
|
|
int start_slot;
|
|
|
|
int num_slots;
|
2014-08-06 14:26:21 +08:00
|
|
|
int vcpi;
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
2017-07-12 23:51:02 +08:00
|
|
|
#define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base)
|
|
|
|
|
drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:
/* We cannot rely on port->vcpi.num_slots to update
* topology_state->avail_slots as the port may not exist if the parent
* branch device was unplugged. This should be fixed by tracking
* per-port slot allocation in drm_dp_mst_topology_state instead of
* depending on the caller to tell us how many slots to release.
*/
That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.
So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.
Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.
Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.
Changes since v9:
* Add some missing changes that were requested by danvet that I forgot
about after I redid all of the kref stuff:
* Remove unnecessary state changes in intel_dp_mst_atomic_check
* Cleanup atomic check logic for VCPI allocations - all we need to check in
compute_config is whether or not this state disables a CRTC, then free
VCPI based off that
Changes since v8:
* Fix compile errors, whoops!
Changes since v7:
- Don't check for mixed stale/valid VCPI allocations, just rely on
connector registration to stop such erroneous modesets
Changes since v6:
- Keep a kref to all of the ports we have allocations on. This required
a good bit of changing to when we call drm_dp_find_vcpi_slots(),
mainly that we need to ensure that we only redo VCPI allocations on
actual mode or CRTC changes, not crtc_state->active changes.
Additionally, we no longer take the registration of the DRM connector
for each port into account because so long as we have a kref to the
port in the new or previous atomic state, the connector will stay
registered.
- Use the small changes to drm_dp_put_port() to add even more error
checking to make misusage of the helpers more obvious. I added this
after having to chase down various use-after-free conditions that
started popping up from the new helpers so no one else has to
troubleshoot that.
- Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
- Update documentation again, note that find/release() should both not be
called on the same port in a single atomic check phase (but multiple
calls to one or the other is OK)
Changes since v4:
- Don't skip the atomic checks for VCPI allocations if no new VCPI
allocations happen in a state. This makes the next change I'm about
to list here a lot easier to implement.
- Don't ignore VCPI allocations on destroyed ports, instead ensure that
when ports are destroyed and still have VCPI allocations in the
topology state, the only state changes allowed are releasing said
ports' VCPI. This prevents a state with a mix of VCPI allocations
from destroyed ports, and allocations from valid ports.
Changes since v3:
- Don't release VCPI allocations in the topology state immediately in
drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
over them in drm_dp_mst_duplicate_state(). This makes it so
drm_dp_atomic_release_vcpi_slots() is still idempotent while also
throwing warnings if the driver messes up it's book keeping and tries
to release VCPI slots on a port that doesn't have any pre-existing
VCPI allocation - danvet
- Change mst_state/state in some debugging messages to "mst state"
Changes since v2:
- Use kmemdup() for duplicating MST state - danvet
- Move port validation out of duplicate state callback - danvet
- Handle looping through MST topology states in
drm_dp_mst_atomic_check() so the driver doesn't have to do it
- Fix documentation in drm_dp_atomic_find_vcpi_slots()
- Move the atomic check for each individual topology state into it's
own function, reduces indenting
- Don't consider "stale" MST ports when calculating the bandwidth
requirements. This is needed because originally we relied on the
state duplication functions to prune any stale ports from the new
state, which would prevent us from incorrectly considering their
bandwidth requirements alongside legitimate new payloads.
- Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
- Annotate atomic VCPI and atomic check functions with __must_check
- danvet
Changes since v1:
- Don't use the now-removed ->atomic_check() for private objects hook,
just give drivers a function to call themselves
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com
2019-01-11 08:53:41 +08:00
|
|
|
struct drm_dp_vcpi_allocation {
|
|
|
|
struct drm_dp_mst_port *port;
|
|
|
|
int vcpi;
|
|
|
|
struct list_head next;
|
|
|
|
};
|
|
|
|
|
2017-04-21 13:51:31 +08:00
|
|
|
struct drm_dp_mst_topology_state {
|
2017-07-12 23:51:02 +08:00
|
|
|
struct drm_private_state base;
|
drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:
/* We cannot rely on port->vcpi.num_slots to update
* topology_state->avail_slots as the port may not exist if the parent
* branch device was unplugged. This should be fixed by tracking
* per-port slot allocation in drm_dp_mst_topology_state instead of
* depending on the caller to tell us how many slots to release.
*/
That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.
So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.
Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.
Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.
Changes since v9:
* Add some missing changes that were requested by danvet that I forgot
about after I redid all of the kref stuff:
* Remove unnecessary state changes in intel_dp_mst_atomic_check
* Cleanup atomic check logic for VCPI allocations - all we need to check in
compute_config is whether or not this state disables a CRTC, then free
VCPI based off that
Changes since v8:
* Fix compile errors, whoops!
Changes since v7:
- Don't check for mixed stale/valid VCPI allocations, just rely on
connector registration to stop such erroneous modesets
Changes since v6:
- Keep a kref to all of the ports we have allocations on. This required
a good bit of changing to when we call drm_dp_find_vcpi_slots(),
mainly that we need to ensure that we only redo VCPI allocations on
actual mode or CRTC changes, not crtc_state->active changes.
Additionally, we no longer take the registration of the DRM connector
for each port into account because so long as we have a kref to the
port in the new or previous atomic state, the connector will stay
registered.
- Use the small changes to drm_dp_put_port() to add even more error
checking to make misusage of the helpers more obvious. I added this
after having to chase down various use-after-free conditions that
started popping up from the new helpers so no one else has to
troubleshoot that.
- Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
- Update documentation again, note that find/release() should both not be
called on the same port in a single atomic check phase (but multiple
calls to one or the other is OK)
Changes since v4:
- Don't skip the atomic checks for VCPI allocations if no new VCPI
allocations happen in a state. This makes the next change I'm about
to list here a lot easier to implement.
- Don't ignore VCPI allocations on destroyed ports, instead ensure that
when ports are destroyed and still have VCPI allocations in the
topology state, the only state changes allowed are releasing said
ports' VCPI. This prevents a state with a mix of VCPI allocations
from destroyed ports, and allocations from valid ports.
Changes since v3:
- Don't release VCPI allocations in the topology state immediately in
drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
over them in drm_dp_mst_duplicate_state(). This makes it so
drm_dp_atomic_release_vcpi_slots() is still idempotent while also
throwing warnings if the driver messes up it's book keeping and tries
to release VCPI slots on a port that doesn't have any pre-existing
VCPI allocation - danvet
- Change mst_state/state in some debugging messages to "mst state"
Changes since v2:
- Use kmemdup() for duplicating MST state - danvet
- Move port validation out of duplicate state callback - danvet
- Handle looping through MST topology states in
drm_dp_mst_atomic_check() so the driver doesn't have to do it
- Fix documentation in drm_dp_atomic_find_vcpi_slots()
- Move the atomic check for each individual topology state into it's
own function, reduces indenting
- Don't consider "stale" MST ports when calculating the bandwidth
requirements. This is needed because originally we relied on the
state duplication functions to prune any stale ports from the new
state, which would prevent us from incorrectly considering their
bandwidth requirements alongside legitimate new payloads.
- Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
- Annotate atomic VCPI and atomic check functions with __must_check
- danvet
Changes since v1:
- Don't use the now-removed ->atomic_check() for private objects hook,
just give drivers a function to call themselves
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com
2019-01-11 08:53:41 +08:00
|
|
|
struct list_head vcpis;
|
2017-04-21 13:51:31 +08:00
|
|
|
struct drm_dp_mst_topology_mgr *mgr;
|
|
|
|
};
|
|
|
|
|
2017-07-12 23:51:02 +08:00
|
|
|
#define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base)
|
|
|
|
|
2014-06-05 12:01:32 +08:00
|
|
|
/**
|
|
|
|
* struct drm_dp_mst_topology_mgr - DisplayPort MST manager
|
|
|
|
*
|
|
|
|
* This struct represents the toplevel displayport MST topology manager.
|
|
|
|
* There should be one instance of this for every MST capable DP connector
|
|
|
|
* on the GPU.
|
|
|
|
*/
|
|
|
|
struct drm_dp_mst_topology_mgr {
|
2017-07-12 23:51:02 +08:00
|
|
|
/**
|
|
|
|
* @base: Base private object for atomic
|
|
|
|
*/
|
|
|
|
struct drm_private_obj base;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @dev: device pointer for adding i2c devices etc.
|
|
|
|
*/
|
2017-01-25 07:49:29 +08:00
|
|
|
struct drm_device *dev;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @cbs: callbacks for connector addition and destruction.
|
|
|
|
*/
|
2015-12-31 05:20:30 +08:00
|
|
|
const struct drm_dp_mst_topology_cbs *cbs;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @max_dpcd_transaction_bytes: maximum number of bytes to read/write
|
|
|
|
* in one go.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
int max_dpcd_transaction_bytes;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @aux: AUX channel for the DP MST connector this topolgy mgr is
|
|
|
|
* controlling.
|
|
|
|
*/
|
|
|
|
struct drm_dp_aux *aux;
|
|
|
|
/**
|
|
|
|
* @max_payloads: maximum number of payloads the GPU can generate.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
int max_payloads;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @conn_base_id: DRM connector ID this mgr is connected to. Only used
|
|
|
|
* to build the MST connector path value.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
int conn_base_id;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2019-09-04 04:45:56 +08:00
|
|
|
* @down_rep_recv: Message receiver state for down replies.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_sideband_msg_rx down_rep_recv;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2019-09-04 04:45:56 +08:00
|
|
|
* @up_req_recv: Message receiver state for up requests.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_sideband_msg_rx up_req_recv;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @lock: protects mst state, primary, dpcd.
|
|
|
|
*/
|
|
|
|
struct mutex lock;
|
2014-06-05 12:01:32 +08:00
|
|
|
|
2019-10-17 04:02:59 +08:00
|
|
|
/**
|
|
|
|
* @probe_lock: Prevents @work and @up_req_work, the only writers of
|
|
|
|
* &drm_dp_mst_port.mstb and &drm_dp_mst_branch.ports, from racing
|
|
|
|
* while they update the topology.
|
|
|
|
*/
|
|
|
|
struct mutex probe_lock;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @mst_state: If this manager is enabled for an MST capable port. False
|
|
|
|
* if no MST sink/branch devices is connected.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
bool mst_state;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @mst_primary: Pointer to the primary/first branch device.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_mst_branch *mst_primary;
|
2016-01-23 06:07:28 +08:00
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @dpcd: Cache of DPCD for primary port.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
u8 dpcd[DP_RECEIVER_CAP_SIZE];
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @sink_count: Sink count from DEVICE_SERVICE_IRQ_VECTOR_ESI0.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
u8 sink_count;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @pbn_div: PBN to slots divisor.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
int pbn_div;
|
2017-03-16 15:10:24 +08:00
|
|
|
|
2017-04-21 13:51:31 +08:00
|
|
|
/**
|
|
|
|
* @funcs: Atomic helper callbacks
|
|
|
|
*/
|
|
|
|
const struct drm_private_state_funcs *funcs;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2017-01-25 14:26:43 +08:00
|
|
|
* @qlock: protects @tx_msg_downq, the &drm_dp_mst_branch.txslost and
|
|
|
|
* &drm_dp_sideband_msg_tx.state once they are queued
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct mutex qlock;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @tx_msg_downq: List of pending down replies.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct list_head tx_msg_downq;
|
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @payload_lock: Protect payload information.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct mutex payload_lock;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @proposed_vcpis: Array of pointers for the new VCPI allocation. The
|
2017-01-25 14:26:43 +08:00
|
|
|
* VCPI structure itself is &drm_dp_mst_port.vcpi.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_vcpi **proposed_vcpis;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @payloads: Array of payloads.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct drm_dp_payload *payloads;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @payload_mask: Elements of @payloads actually in use. Since
|
|
|
|
* reallocation of active outputs isn't possible gaps can be created by
|
|
|
|
* disabling outputs out of order compared to how they've been enabled.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
unsigned long payload_mask;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @vcpi_mask: Similar to @payload_mask, but for @proposed_vcpis.
|
|
|
|
*/
|
2014-08-06 14:26:21 +08:00
|
|
|
unsigned long vcpi_mask;
|
2014-06-05 12:01:32 +08:00
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @tx_waitq: Wait to queue stall for the tx worker.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
wait_queue_head_t tx_waitq;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @work: Probe work.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct work_struct work;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
|
|
|
* @tx_work: Sideband transmit worker. This can nest within the main
|
|
|
|
* @work worker for each transaction @work launches.
|
|
|
|
*/
|
2014-06-05 12:01:32 +08:00
|
|
|
struct work_struct tx_work;
|
2015-06-15 08:34:28 +08:00
|
|
|
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2019-02-20 06:41:02 +08:00
|
|
|
* @destroy_port_list: List of to be destroyed connectors.
|
|
|
|
*/
|
|
|
|
struct list_head destroy_port_list;
|
|
|
|
/**
|
|
|
|
* @destroy_branch_device_list: List of to be destroyed branch
|
|
|
|
* devices.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2019-02-20 06:41:02 +08:00
|
|
|
struct list_head destroy_branch_device_list;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2019-02-20 06:41:02 +08:00
|
|
|
* @delayed_destroy_lock: Protects @destroy_port_list and
|
|
|
|
* @destroy_branch_device_list.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2019-02-20 06:41:02 +08:00
|
|
|
struct mutex delayed_destroy_lock;
|
2016-07-16 03:48:04 +08:00
|
|
|
/**
|
2019-02-20 06:41:02 +08:00
|
|
|
* @delayed_destroy_work: Work item to destroy MST port and branch
|
|
|
|
* devices, needed to avoid locking inversion.
|
2016-07-16 03:48:04 +08:00
|
|
|
*/
|
2019-02-20 06:41:02 +08:00
|
|
|
struct work_struct delayed_destroy_work;
|
drm/dp_mst: Handle UP requests asynchronously
Once upon a time, hotplugging devices on MST branches actually worked in
DRM. Now, it only works in amdgpu (likely because of how it's hotplug
handlers are implemented). On both i915 and nouveau, hotplug
notifications from MST branches are noticed - but trying to respond to
them causes messaging timeouts and causes the whole topology state to go
out of sync with reality, usually resulting in the user needing to
replug the entire topology in hopes that it actually fixes things.
The reason for this is because the way we currently handle UP requests
in MST is completely bogus. drm_dp_mst_handle_up_req() is called from
drm_dp_mst_hpd_irq(), which is usually called from the driver's hotplug
handler. Because we handle sending the hotplug event from this function,
we actually cause the driver's hotplug handler (and in turn, all
sideband transactions) to block on
drm_device->mode_config.connection_mutex. This makes it impossible to
send any sideband messages from the driver's connector probing
functions, resulting in the aforementioned sideband message timeout.
There's even more problems with this beyond breaking hotplugging on MST
branch devices. It also makes it almost impossible to protect
drm_dp_mst_port struct members under a lock because we then have to
worry about dealing with all of the lock dependency issues that ensue.
So, let's finally actually fix this issue by handling the processing of
up requests asyncronously. This way we can send sideband messages from
most contexts without having to deal with getting blocked if we hold
connection_mutex. This also fixes MST branch device hotplugging on i915,
finally!
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-5-lyude@redhat.com
2019-06-18 04:37:18 +08:00
|
|
|
|
|
|
|
/**
|
|
|
|
* @up_req_list: List of pending up requests from the topology that
|
|
|
|
* need to be processed, in chronological order.
|
|
|
|
*/
|
|
|
|
struct list_head up_req_list;
|
|
|
|
/**
|
|
|
|
* @up_req_lock: Protects @up_req_list
|
|
|
|
*/
|
|
|
|
struct mutex up_req_lock;
|
|
|
|
/**
|
|
|
|
* @up_req_work: Work item to process up requests received from the
|
|
|
|
* topology. Needed to avoid blocking hotplug handling and sideband
|
|
|
|
* transmissions.
|
|
|
|
*/
|
|
|
|
struct work_struct up_req_work;
|
2014-06-05 12:01:32 +08:00
|
|
|
};
|
|
|
|
|
2017-01-25 07:49:29 +08:00
|
|
|
int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_device *dev, struct drm_dp_aux *aux,
|
|
|
|
int max_dpcd_transaction_bytes,
|
|
|
|
int max_payloads, int conn_base_id);
|
2014-06-05 12:01:32 +08:00
|
|
|
|
|
|
|
void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handled);
|
|
|
|
|
|
|
|
|
drm/dp_mst: Protect drm_dp_mst_port members with locking
This is a complicated one. Essentially, there's currently a problem in the MST
core that hasn't really caused any issues that we're aware of (emphasis on "that
we're aware of"): locking.
When we go through and probe the link addresses and path resources in a
topology, we hold no locks when updating ports with said information. The
members I'm referring to in particular are:
- ldps
- ddps
- mcs
- pdt
- dpcd_rev
- num_sdp_streams
- num_sdp_stream_sinks
- available_pbn
- input
- connector
Now that we're handling UP requests asynchronously and will be using some of
the struct members mentioned above in atomic modesetting in the future for
features such as PBN validation, this is going to become a lot more important.
As well, the next few commits that prepare us for and introduce suspend/resume
reprobing will also need clear locking in order to prevent from additional
racing hilarities that we never could have hit in the past.
So, let's solve this issue by using &mgr->base.lock, the modesetting
lock which currently only protects &mgr->base.state. This works
perfectly because it allows us to avoid blocking connection_mutex
unnecessarily, and we can grab this in connector detection paths since
it's a ww mutex. We start by having drm_dp_mst_handle_up_req() hold this
when updating ports. For drm_dp_mst_handle_link_address_port() things
are a bit more complicated. As I've learned the hard way, we can grab
&mgr->lock.base for everything except for port->connector. See, our
normal driver probing paths end up generating this rather obvious
lockdep chain:
&drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
However, sysfs grabs &drm->mode_config.mutex in order to protect itself
from connector state changing under it. Because this entails grabbing
kn->count, e.g. the lock that the kernel provides for protecting sysfs
contexts, we end up grabbing kn->count followed by
&drm->mode_config.mutex. This ends up creating an extremely rude chain:
&kn->count
-> &drm->mode_config.mutex
-> crtc_ww_class_mutex/crtc_ww_class_acquire
-> &connector->mutex
I mean, look at that thing! It's just evil!!! This gross thing ends up
making any calls to drm_connector_register()/drm_connector_unregister()
impossible when holding any kind of modesetting lock. This is annoying
because ideally, we always want to ensure that
drm_dp_mst_port->connector never changes when doing an atomic commit or
check that would affect the atomic topology state so that it can
reliably and easily be used from future DRM DP MST helpers to assist
with tasks such as scanning through the current VCPI allocations and
adding connectors which need to have their allocations updated in
response to a bandwidth change or the like.
Being able to hold &mgr->base.lock throughout the entire link probe
process would have been _great_, since we could prevent userspace from
ever seeing any states in-between individual port changes and as a
result likely end up with a much faster probe and more consistent
results from said probes. But without some rework of how we handle
connector probing in sysfs it's not at all currently possible. In the
future, maybe we can try using the sysfs locks to protect updates to
connector probing state and fix this mess.
So for now, to protect everything other than port->connector under
&mgr->base.lock and ensure that we still have the guarantee that atomic
check/commit contexts will never see port->connector change we use a
silly trick. See: port->connector only needs to change in order to
ensure that input ports (see the MST spec) never have a ghost connector
associated with them. But, there's nothing stopping us from simply
throwing the entire port out and creating a new one in order to maintain
that requirement while still keeping port->connector consistent across
the lifetime of the port in atomic check/commit contexts. For all
intended purposes this works fine, as we validate ports in any contexts
we care about before using them and as such will end up reporting the
connector as disconnected until it's port's destruction finalizes. So,
we just do that in cases where we detect port->input has transitioned
from true->false. We don't need to worry about the other direction,
since a port without a connector isn't visible to userspace and as such
doesn't need to be protected by &mgr->base.lock until we finish
registering a connector for it.
For updating members of drm_dp_mst_port other than port->connector, we
simply grab &mgr->base.lock in drm_dp_mst_link_probe_work() for already
registered ports, update said members and drop the lock before
potentially registering a connector and probing the link address of it's
children.
Finally, we modify drm_dp_mst_detect_port() to take a modesetting lock
acquisition context in order to acquire &mgr->base.lock under
&connection_mutex and convert all it's users over to using the
.detect_ctx probe hooks.
With that, we finally have well defined locking.
Changes since v4:
* Get rid of port->mutex, stop using connection_mutex and just use our own
modesetting lock - mgr->base.lock. Also, add a probe_lock that comes
before this patch.
* Just throw out ports that get changed from an output to an input, and
replace them with new ports. This lets us ensure that modesetting
contexts never see port->connector go from having a connector to being
NULL.
* Write an extremely detailed explanation of what problems this is
trying to fix, since there's a _lot_ of context here and I honestly
forgot some of it myself a couple times.
* Don't grab mgr->lock when reading port->mstb in
drm_dp_mst_handle_link_address_port(). It's not needed.
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-7-lyude@redhat.com
2019-06-18 05:59:29 +08:00
|
|
|
int
|
|
|
|
drm_dp_mst_detect_port(struct drm_connector *connector,
|
|
|
|
struct drm_modeset_acquire_ctx *ctx,
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port);
|
2014-06-05 12:01:32 +08:00
|
|
|
|
2015-12-02 14:09:43 +08:00
|
|
|
bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port);
|
2014-06-05 12:01:32 +08:00
|
|
|
struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_calc_pbn_mode(int clock, int bpp);
|
|
|
|
|
|
|
|
|
2017-03-16 15:10:26 +08:00
|
|
|
bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port, int pbn, int slots);
|
2014-06-05 12:01:32 +08:00
|
|
|
|
2015-02-24 07:23:55 +08:00
|
|
|
int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
|
|
|
|
|
2014-06-05 12:01:32 +08:00
|
|
|
|
|
|
|
void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
|
|
|
|
|
|
|
|
|
|
|
|
void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
int pbn);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
|
|
|
|
|
|
|
|
|
|
|
|
int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr);
|
|
|
|
|
|
|
|
int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr);
|
|
|
|
|
|
|
|
void drm_dp_mst_dump_topology(struct seq_file *m,
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr);
|
|
|
|
|
|
|
|
void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
|
2019-01-09 05:11:29 +08:00
|
|
|
int __must_check
|
drm/dp_mst: Add basic topology reprobing when resuming
Finally! For a very long time, our MST helpers have had one very
annoying issue: They don't know how to reprobe the topology state when
coming out of suspend. This means that if a user has a machine connected
to an MST topology and decides to suspend their machine, we lose all
topology changes that happened during that period. That can be a big
problem if the machine was connected to a different topology on the same
port before resuming, as we won't bother reprobing any of the ports and
likely cause the user's monitors not to come back up as expected.
So, we start fixing this by teaching our MST helpers how to reprobe the
link addresses of each connected topology when resuming. As it turns
out, the behavior that we want here is identical to the behavior we want
when initially probing a newly connected MST topology, with a couple of
important differences:
- We need to be more careful about handling the potential races between
events from the MST hub that could change the topology state as we're
performing the link address reprobe
- We need to be more careful about handling unlikely state changes on
ports - such as an input port turning into an output port, something
that would be far more likely to happen in situations like the MST hub
we're connected to being changed while we're suspend
Both of which have been solved by previous commits. That leaves one
requirement:
- We need to prune any MST ports in our in-memory topology state that
were present when suspending, but have not appeared in the post-resume
link address response from their parent branch device
Which we can now handle in this commit by modifying
drm_dp_send_link_address(). We then introduce suspend/resume reprobing
by introducing drm_dp_mst_topology_mgr_invalidate_mstb(), which we call
in drm_dp_mst_topology_mgr_suspend() to traverse the in-memory topology
state to indicate that each mstb needs it's link address resent and PBN
resources reprobed.
On resume, we start back up &mgr->work and have it reprobe the topology
in the same way we would on a hotplug, removing any leftover ports that
no longer appear in the topology state.
Changes since v4:
* Split indenting changes in drm_dp_mst_topology_mgr_resume() into a
separate patch
* Only fire hotplugs when something has actually changed after a link
address probe
* Don't try to change port->connector at all on ports, just throw out
ports that need their connectors removed to make things easier.
Cc: Juston Li <juston.li@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <sean@poorly.run>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-14-lyude@redhat.com
2019-06-18 07:57:33 +08:00
|
|
|
drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
bool sync);
|
drm/dp_mst: Enable registration of AUX devices for MST ports
All available downstream ports - physical and logical - are exposed for
each MST device. They are listed in /dev/, following the same naming
scheme as SST devices by appending an incremental ID.
Although all downstream ports are exposed, only some will work as
expected. Consider the following topology:
+---------+
| ASIC |
+---------+
Conn-0|
|
+----v----+
+----| MST HUB |----+
| +---------+ |
| |
|Port-1 Port-2|
+-----v-----+ +-----v-----+
| MST | | SST |
| Display | | Display |
+-----------+ +-----------+
|Port-1
x
MST Path | MST Device
----------+----------------------------------
sst:0 | MST Hub
mst:0-1 | MST Display
mst:0-1-1 | MST Display's disconnected DP out
mst:0-1-8 | MST Display's internal sink
mst:0-2 | SST Display
On certain MST displays, the upstream physical port will ACK DPCD reads.
However, reads on the local logical port to the internal sink will
*NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs.
There may also be duplicates. Some displays will return the same GUID
when reading DPCD from both mst:0-1 and mst:0-1-8.
There are some device-dependent behavior as well. The MST hub used
during testing will actually *ACK* read requests on a disconnected
physical port, whereas the MST displays will NAK.
In light of these discrepancies, it's simpler to expose all downstream
ports - both physical and logical - and let the user decide what to use.
v3 changes:
* Change WARN_ON_ONCE -> DRM_ERROR on dpcd read errors
* Docstring and cosmetic fixes
v2 changes:
Moved remote aux device (un)registration to new mst connector late
register and early unregister helpers. Drivers should call these from
their own mst connector function hooks.
This is to solve an issue during driver unload, where mst connector
devices are unregistered before the remote aux devices are. In a setup
where aux devices are created as children of connector devices, the aux
device would be removed too early, and uncleanly. Doing so in
early_unregister solves this issue, as that is called before connector
unregistration.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Leo Li <sunpeng.li@amd.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190723232808.28128-3-sunpeng.li@amd.com
2019-07-24 07:28:01 +08:00
|
|
|
|
|
|
|
ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
|
|
|
|
unsigned int offset, void *buffer, size_t size);
|
|
|
|
ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
|
|
|
|
unsigned int offset, void *buffer, size_t size);
|
|
|
|
|
|
|
|
int drm_dp_mst_connector_late_register(struct drm_connector *connector,
|
|
|
|
struct drm_dp_mst_port *port);
|
|
|
|
void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
|
|
|
|
struct drm_dp_mst_port *port);
|
|
|
|
|
2017-04-21 13:51:31 +08:00
|
|
|
struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr);
|
drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:
/* We cannot rely on port->vcpi.num_slots to update
* topology_state->avail_slots as the port may not exist if the parent
* branch device was unplugged. This should be fixed by tracking
* per-port slot allocation in drm_dp_mst_topology_state instead of
* depending on the caller to tell us how many slots to release.
*/
That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.
So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.
Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.
Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.
Changes since v9:
* Add some missing changes that were requested by danvet that I forgot
about after I redid all of the kref stuff:
* Remove unnecessary state changes in intel_dp_mst_atomic_check
* Cleanup atomic check logic for VCPI allocations - all we need to check in
compute_config is whether or not this state disables a CRTC, then free
VCPI based off that
Changes since v8:
* Fix compile errors, whoops!
Changes since v7:
- Don't check for mixed stale/valid VCPI allocations, just rely on
connector registration to stop such erroneous modesets
Changes since v6:
- Keep a kref to all of the ports we have allocations on. This required
a good bit of changing to when we call drm_dp_find_vcpi_slots(),
mainly that we need to ensure that we only redo VCPI allocations on
actual mode or CRTC changes, not crtc_state->active changes.
Additionally, we no longer take the registration of the DRM connector
for each port into account because so long as we have a kref to the
port in the new or previous atomic state, the connector will stay
registered.
- Use the small changes to drm_dp_put_port() to add even more error
checking to make misusage of the helpers more obvious. I added this
after having to chase down various use-after-free conditions that
started popping up from the new helpers so no one else has to
troubleshoot that.
- Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
- Update documentation again, note that find/release() should both not be
called on the same port in a single atomic check phase (but multiple
calls to one or the other is OK)
Changes since v4:
- Don't skip the atomic checks for VCPI allocations if no new VCPI
allocations happen in a state. This makes the next change I'm about
to list here a lot easier to implement.
- Don't ignore VCPI allocations on destroyed ports, instead ensure that
when ports are destroyed and still have VCPI allocations in the
topology state, the only state changes allowed are releasing said
ports' VCPI. This prevents a state with a mix of VCPI allocations
from destroyed ports, and allocations from valid ports.
Changes since v3:
- Don't release VCPI allocations in the topology state immediately in
drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
over them in drm_dp_mst_duplicate_state(). This makes it so
drm_dp_atomic_release_vcpi_slots() is still idempotent while also
throwing warnings if the driver messes up it's book keeping and tries
to release VCPI slots on a port that doesn't have any pre-existing
VCPI allocation - danvet
- Change mst_state/state in some debugging messages to "mst state"
Changes since v2:
- Use kmemdup() for duplicating MST state - danvet
- Move port validation out of duplicate state callback - danvet
- Handle looping through MST topology states in
drm_dp_mst_atomic_check() so the driver doesn't have to do it
- Fix documentation in drm_dp_atomic_find_vcpi_slots()
- Move the atomic check for each individual topology state into it's
own function, reduces indenting
- Don't consider "stale" MST ports when calculating the bandwidth
requirements. This is needed because originally we relied on the
state duplication functions to prune any stale ports from the new
state, which would prevent us from incorrectly considering their
bandwidth requirements alongside legitimate new payloads.
- Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
- Annotate atomic VCPI and atomic check functions with __must_check
- danvet
Changes since v1:
- Don't use the now-removed ->atomic_check() for private objects hook,
just give drivers a function to call themselves
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com
2019-01-11 08:53:41 +08:00
|
|
|
int __must_check
|
|
|
|
drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port, int pbn);
|
|
|
|
int __must_check
|
|
|
|
drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state,
|
|
|
|
struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port);
|
2017-09-07 08:14:58 +08:00
|
|
|
int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
|
|
|
|
struct drm_dp_mst_port *port, bool power_up);
|
drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:
/* We cannot rely on port->vcpi.num_slots to update
* topology_state->avail_slots as the port may not exist if the parent
* branch device was unplugged. This should be fixed by tracking
* per-port slot allocation in drm_dp_mst_topology_state instead of
* depending on the caller to tell us how many slots to release.
*/
That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.
So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.
Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.
Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.
Changes since v9:
* Add some missing changes that were requested by danvet that I forgot
about after I redid all of the kref stuff:
* Remove unnecessary state changes in intel_dp_mst_atomic_check
* Cleanup atomic check logic for VCPI allocations - all we need to check in
compute_config is whether or not this state disables a CRTC, then free
VCPI based off that
Changes since v8:
* Fix compile errors, whoops!
Changes since v7:
- Don't check for mixed stale/valid VCPI allocations, just rely on
connector registration to stop such erroneous modesets
Changes since v6:
- Keep a kref to all of the ports we have allocations on. This required
a good bit of changing to when we call drm_dp_find_vcpi_slots(),
mainly that we need to ensure that we only redo VCPI allocations on
actual mode or CRTC changes, not crtc_state->active changes.
Additionally, we no longer take the registration of the DRM connector
for each port into account because so long as we have a kref to the
port in the new or previous atomic state, the connector will stay
registered.
- Use the small changes to drm_dp_put_port() to add even more error
checking to make misusage of the helpers more obvious. I added this
after having to chase down various use-after-free conditions that
started popping up from the new helpers so no one else has to
troubleshoot that.
- Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
- Update documentation again, note that find/release() should both not be
called on the same port in a single atomic check phase (but multiple
calls to one or the other is OK)
Changes since v4:
- Don't skip the atomic checks for VCPI allocations if no new VCPI
allocations happen in a state. This makes the next change I'm about
to list here a lot easier to implement.
- Don't ignore VCPI allocations on destroyed ports, instead ensure that
when ports are destroyed and still have VCPI allocations in the
topology state, the only state changes allowed are releasing said
ports' VCPI. This prevents a state with a mix of VCPI allocations
from destroyed ports, and allocations from valid ports.
Changes since v3:
- Don't release VCPI allocations in the topology state immediately in
drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
over them in drm_dp_mst_duplicate_state(). This makes it so
drm_dp_atomic_release_vcpi_slots() is still idempotent while also
throwing warnings if the driver messes up it's book keeping and tries
to release VCPI slots on a port that doesn't have any pre-existing
VCPI allocation - danvet
- Change mst_state/state in some debugging messages to "mst state"
Changes since v2:
- Use kmemdup() for duplicating MST state - danvet
- Move port validation out of duplicate state callback - danvet
- Handle looping through MST topology states in
drm_dp_mst_atomic_check() so the driver doesn't have to do it
- Fix documentation in drm_dp_atomic_find_vcpi_slots()
- Move the atomic check for each individual topology state into it's
own function, reduces indenting
- Don't consider "stale" MST ports when calculating the bandwidth
requirements. This is needed because originally we relied on the
state duplication functions to prune any stale ports from the new
state, which would prevent us from incorrectly considering their
bandwidth requirements alongside legitimate new payloads.
- Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
- Annotate atomic VCPI and atomic check functions with __must_check
- danvet
Changes since v1:
- Don't use the now-removed ->atomic_check() for private objects hook,
just give drivers a function to call themselves
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com
2019-01-11 08:53:41 +08:00
|
|
|
int __must_check drm_dp_mst_atomic_check(struct drm_atomic_state *state);
|
2017-04-21 13:51:31 +08:00
|
|
|
|
drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.
To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:
commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;
commit 263efde31f97 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.
Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:
commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref")
But then that introduced use-after-free errors, so I quickly reverted
it:
commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"")
And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortunately, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.
After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.
To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.
Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.
Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:
- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)
Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.
Changes since v4:
* Change the kernel-figure summary for dp-mst/topology-figure-1.dot a
bit - danvet
* Remove figure numbers - danvet
Changes since v3:
* Remove rebase detritus - danvet
* Split out purely style changes into separate patches - hwentlan
Changes since v2:
* Fix commit message - checkpatch
* s/)-1/) - 1/g - checkpatch
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
and payloads - danvet
* Make suggested documentation changes for
drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() -
danvet
* Better explain the relationship between malloc refs and topology krefs
in the documentation for drm_dp_mst_topology_get_port() and
drm_dp_mst_topology_get_mstb() - danvet
* Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet
* Rename drm_dp_mst_topology_get_(port|mstb)() ->
drm_dp_mst_topology_try_get_(port|mstb)() and
drm_dp_mst_topology_ref_(port|mstb)() ->
drm_dp_mst_topology_get_(port|mstb)() - danvet
* s/should/must in docs - danvet
* WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet
* Move kdocs for mstb/port structs inline - danvet
* Split drm_dp_get_last_connected_port_and_mstb() changes into their own
commit - danvet
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
Cc: Juston Li <juston.li@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com
2019-01-11 08:53:29 +08:00
|
|
|
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);
|
|
|
|
|
2019-01-11 08:53:40 +08:00
|
|
|
extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
|
|
|
|
|
|
|
|
/**
|
|
|
|
* __drm_dp_mst_state_iter_get - private atomic state iterator function for
|
|
|
|
* macro-internal use
|
|
|
|
* @state: &struct drm_atomic_state pointer
|
|
|
|
* @mgr: pointer to the &struct drm_dp_mst_topology_mgr iteration cursor
|
|
|
|
* @old_state: optional pointer to the old &struct drm_dp_mst_topology_state
|
|
|
|
* iteration cursor
|
|
|
|
* @new_state: optional pointer to the new &struct drm_dp_mst_topology_state
|
|
|
|
* iteration cursor
|
|
|
|
* @i: int iteration cursor, for macro-internal use
|
|
|
|
*
|
|
|
|
* Used by for_each_oldnew_mst_mgr_in_state(),
|
|
|
|
* for_each_old_mst_mgr_in_state(), and for_each_new_mst_mgr_in_state(). Don't
|
|
|
|
* call this directly.
|
|
|
|
*
|
|
|
|
* Returns:
|
|
|
|
* True if the current &struct drm_private_obj is a &struct
|
|
|
|
* drm_dp_mst_topology_mgr, false otherwise.
|
|
|
|
*/
|
|
|
|
static inline bool
|
|
|
|
__drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
|
|
|
|
struct drm_dp_mst_topology_mgr **mgr,
|
|
|
|
struct drm_dp_mst_topology_state **old_state,
|
|
|
|
struct drm_dp_mst_topology_state **new_state,
|
|
|
|
int i)
|
|
|
|
{
|
|
|
|
struct __drm_private_objs_state *objs_state = &state->private_objs[i];
|
|
|
|
|
|
|
|
if (objs_state->ptr->funcs != &drm_dp_mst_topology_state_funcs)
|
|
|
|
return false;
|
|
|
|
|
|
|
|
*mgr = to_dp_mst_topology_mgr(objs_state->ptr);
|
|
|
|
if (old_state)
|
|
|
|
*old_state = to_dp_mst_topology_state(objs_state->old_state);
|
|
|
|
if (new_state)
|
|
|
|
*new_state = to_dp_mst_topology_state(objs_state->new_state);
|
|
|
|
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
/**
|
|
|
|
* for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology
|
|
|
|
* managers in an atomic update
|
|
|
|
* @__state: &struct drm_atomic_state pointer
|
|
|
|
* @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
|
|
|
|
* @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
|
|
|
|
* state
|
|
|
|
* @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
|
|
|
|
* state
|
|
|
|
* @__i: int iteration cursor, for macro-internal use
|
|
|
|
*
|
|
|
|
* This iterates over all DRM DP MST topology managers in an atomic update,
|
|
|
|
* tracking both old and new state. This is useful in places where the state
|
|
|
|
* delta needs to be considered, for example in atomic check functions.
|
|
|
|
*/
|
|
|
|
#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state, __i) \
|
|
|
|
for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
|
|
|
|
for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), &(old_state), &(new_state), (__i)))
|
|
|
|
|
|
|
|
/**
|
|
|
|
* for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers
|
|
|
|
* in an atomic update
|
|
|
|
* @__state: &struct drm_atomic_state pointer
|
|
|
|
* @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
|
|
|
|
* @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
|
|
|
|
* state
|
|
|
|
* @__i: int iteration cursor, for macro-internal use
|
|
|
|
*
|
|
|
|
* This iterates over all DRM DP MST topology managers in an atomic update,
|
|
|
|
* tracking only the old state. This is useful in disable functions, where we
|
|
|
|
* need the old state the hardware is still in.
|
|
|
|
*/
|
|
|
|
#define for_each_old_mst_mgr_in_state(__state, mgr, old_state, __i) \
|
|
|
|
for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
|
|
|
|
for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), &(old_state), NULL, (__i)))
|
|
|
|
|
|
|
|
/**
|
|
|
|
* for_each_new_mst_mgr_in_state - iterate over all DP MST topology managers
|
|
|
|
* in an atomic update
|
|
|
|
* @__state: &struct drm_atomic_state pointer
|
|
|
|
* @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
|
|
|
|
* @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
|
|
|
|
* state
|
|
|
|
* @__i: int iteration cursor, for macro-internal use
|
|
|
|
*
|
|
|
|
* This iterates over all DRM DP MST topology managers in an atomic update,
|
|
|
|
* tracking only the new state. This is useful in enable functions, where we
|
|
|
|
* need the new state the hardware should be in when the atomic commit
|
|
|
|
* operation has completed.
|
|
|
|
*/
|
|
|
|
#define for_each_new_mst_mgr_in_state(__state, mgr, new_state, __i) \
|
|
|
|
for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
|
|
|
|
for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), NULL, &(new_state), (__i)))
|
|
|
|
|
2014-06-05 12:01:32 +08:00
|
|
|
#endif
|