[PATCH 2/2] drm/dp_mst: Register aux-dev nodes for MST ports
Lyude Paul
lyude at redhat.com
Tue Apr 16 22:22:33 UTC 2019
On Fri, 2019-04-12 at 12:05 -0400, sunpeng.li at amd.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Expose AUX devices for MST ports, similar to how they are exposed for
> SST.
>
> The registered device will have it's MST port path appended in order to
> identify it. i.e. /dev/drm_dp_aux4_mst:0-2-1
>
> So for a MST topology like so:
>
> +---------+
> | ASIC |
> +---------+
> Conn-0|
> |
> +----v----+
> +----| MST HUB |----+
> | +---------+ |
> | |
> |Port-1 Port-2|
> +-----v-----+ +-----v-----+
> | MST | | SST |
> | Display | | Display |
> +-----------+ +-----------+
> |Port-1
> x
>
> The list of AUX device names will look like:
>
> AUX Device Name | MST Device
> ----------------------+----------------------------------
> drm_dp_aux0 | MST Hub
> drm_dp_aux1_mst:0-1-1 | MST Display's disconnected DP out
> drm_dp_aux2_mst:0-1 | MST Display
> drm_dp_aux3_mst:0-2 | SST Display
>
> Note that aux devices are only created for Physical Ports. Logical Ports
> are left out, since they are internally connected within the MST device
> (not connected to a DP RX or TX).
>
> Leo Li:
> * Add missing drm_crtc_helper_internal.h include
> * Fix hard-coded offset and size in drm_dp_send_dpcd_read()
> * Only create aux devices for physical ports.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Leo Li <sunpeng.li at amd.com>
> ---
> drivers/gpu/drm/drm_dp_aux_dev.c | 13 +++-
> drivers/gpu/drm/drm_dp_mst_topology.c | 109 ++++++++++++++++++++++++++++++-
> ---
> include/drm/drm_dp_helper.h | 4 ++
> include/drm/drm_dp_mst_helper.h | 6 ++
> 4 files changed, 117 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 2310a67..f1241d1 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -34,6 +34,7 @@
> #include <linux/uaccess.h>
> #include <linux/uio.h>
> #include <drm/drm_dp_helper.h>
> +#include <drm/drm_dp_mst_helper.h>
> #include <drm/drm_crtc.h>
> #include <drm/drmP.h>
>
> @@ -160,7 +161,11 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb,
> struct iov_iter *to)
> break;
> }
>
> - res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> + if (aux_dev->aux->is_remote)
> + res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
> todo);
> + else
> + res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +
mhhhh, is there a reason we can't just use the normal ->read and ->write
auxdev callbacks for this instead of mixing layers like this?
> if (res <= 0)
> break;
>
> @@ -207,7 +212,11 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
> break;
> }
>
> - res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> + if (aux_dev->aux->is_remote)
> + res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
> todo);
> + else
> + res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +
> if (res <= 0)
> break;
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2ab16c9..d5282db 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -35,6 +35,8 @@
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
>
> +#include "drm_crtc_helper_internal.h"
> +
> /**
> * DOC: dp mst helper
> *
> @@ -52,6 +54,9 @@ static int drm_dp_dpcd_write_payload(struct
> drm_dp_mst_topology_mgr *mgr,
> int id,
> struct drm_dp_payload *payload);
>
> +static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_mst_port *port,
> + int offset, int size, u8 *bytes);
> static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> int offset, int size, u8 *bytes);
> @@ -941,6 +946,8 @@ static void drm_dp_destroy_port(struct kref *kref)
> struct drm_dp_mst_topology_mgr *mgr = port->mgr;
>
> if (!port->input) {
> + drm_dp_aux_unregister_devnode(&port->aux);
> +
> port->vcpi.num_slots = 0;
>
> kfree(port->cached_edid);
> @@ -1095,6 +1102,46 @@ static bool drm_dp_port_setup_pdt(struct
> drm_dp_mst_port *port)
> return send_link;
> }
>
> +/**
> + * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via
> sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to read
> + * @buffer: buffer to store the register values
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_read() does for local
> + * devices via actual AUX CH.
> + */
> +ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> + unsigned int offset, void *buffer, size_t size)
> +{
> + struct drm_dp_mst_port *port = container_of(aux, struct
> drm_dp_mst_port, aux);
> +
> + return drm_dp_send_dpcd_read(port->mgr, port,
> + offset, size, buffer);
> +}
> +
> +/**
> + * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via
> sideband
> + * @aux: Fake sideband AUX CH
> + * @offset: address of the (first) register to write
> + * @buffer: buffer containing the values to write
> + * @size: number of bytes in @buffer
> + *
> + * Performs the same functionality for remote devices via
> + * sideband messaging as drm_dp_dpcd_write() does for local
> + * devices via actual AUX CH.
> + */
> +ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> + unsigned int offset, void *buffer, size_t size)
> +{
> + struct drm_dp_mst_port *port = container_of(aux, struct
> drm_dp_mst_port, aux);
> +
> + return drm_dp_send_dpcd_write(port->mgr, port,
> + offset, size, buffer);
> +}
> +
> static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8
> *guid)
> {
> int ret;
> @@ -1158,6 +1205,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
> port->mgr = mstb->mgr;
> port->aux.name = "DPMST";
> port->aux.dev = dev->dev;
> + port->aux.is_remote = true;
> created = true;
> } else {
> old_pdt = port->pdt;
> @@ -1188,7 +1236,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
> drm_dp_send_enum_path_resources(mstb->mgr,
> mstb, port);
> } else {
> port->available_pbn = 0;
> - }
> + }
> }
>
> if (old_pdt != port->pdt && !port->input) {
> @@ -1220,6 +1268,14 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
> drm_connector_set_tile_property(port->connector);
> }
> (*mstb->mgr->cbs->register_connector)(port->connector);
> +
> + /* Prepend an '_' in front to suffix the aux device filename
> */
> + memmove(proppath + 1, proppath, sizeof(proppath) - 1);
> + proppath[0] = '_';
> +
> + /* Only create aux devices for physical ports with a TX or
> RX*/
> + if (port->port_num < DP_MST_LOGICAL_PORT_0)
> + drm_dp_aux_register_devnode(&port->aux, proppath);
> }
>
> out:
> @@ -1404,7 +1460,6 @@ static bool drm_dp_validate_guid(struct
> drm_dp_mst_topology_mgr *mgr,
> return false;
> }
>
> -#if 0
> static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num,
> u32 offset, u8 num_bytes)
> {
> struct drm_dp_sideband_msg_req_body req;
> @@ -1417,7 +1472,6 @@ static int build_dpcd_read(struct
> drm_dp_sideband_msg_tx *msg, u8 port_num, u32
>
> return 0;
> }
> -#endif
>
> static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> bool up, u8 *msg, int len)
> @@ -1994,26 +2048,55 @@ int drm_dp_update_payload_part2(struct
> drm_dp_mst_topology_mgr *mgr)
> }
> EXPORT_SYMBOL(drm_dp_update_payload_part2);
>
> -#if 0 /* unused as of yet */
> static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> - int offset, int size)
> + int offset, int size, u8 *bytes)
> {
> int len;
> + int ret = 0;
> struct drm_dp_sideband_msg_tx *txmsg;
> + struct drm_dp_mst_branch *mstb;
> +
> + mstb = drm_dp_get_validated_mstb_ref(mgr, port->parent);
> + if (!mstb)
> + return -EINVAL;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> - if (!txmsg)
> - return -ENOMEM;
> + if (!txmsg) {
> + ret = -ENOMEM;
> + goto fail_put;
> + }
>
> - len = build_dpcd_read(txmsg, port->port_num, 0, 8);
> + len = build_dpcd_read(txmsg, port->port_num, offset, size);
> txmsg->dst = port->parent;
>
> drm_dp_queue_down_tx(mgr, txmsg);
>
> - return 0;
> + ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
> + if (ret < 0)
> + goto fail_free;
> +
> + /* DPCD read should never be NACKed */
> + if (WARN_ON_ONCE(txmsg->reply.reply_type == 1)) {
> + ret = -EIO;
> + goto fail_free;
> + }
> +
> + if (txmsg->reply.u.remote_dpcd_read_ack.num_bytes != size) {
> + ret = -EPROTO;
> + goto fail_free;
> + }
> +
> + ret = min_t(size_t, txmsg->reply.u.remote_dpcd_read_ack.num_bytes,
> size);
> + memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, ret);
> +
> +fail_free:
> + kfree(txmsg);
> +fail_put:
> + drm_dp_put_mst_branch_device(mstb);
> +
> + return ret;
> }
> -#endif
>
> static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> @@ -2041,9 +2124,9 @@ static int drm_dp_send_dpcd_write(struct
> drm_dp_mst_topology_mgr *mgr,
>
> ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
> if (ret > 0) {
> - if (txmsg->reply.reply_type == 1) {
> - ret = -EINVAL;
> - } else
> + if (txmsg->reply.reply_type == 1)
> + ret = -EIO;
> + else
> ret = 0;
> }
> kfree(txmsg);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 509667e..6dea76a 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1265,6 +1265,10 @@ struct drm_dp_aux {
> * @cec: struct containing fields used for CEC-Tunneling-over-AUX.
> */
> struct drm_dp_aux_cec cec;
> + /**
> + * @is_remote: Is this "AUX CH" actually using sideband messaging.
> + */
> + bool is_remote;
> };
>
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 371cc28..30f8c11 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -615,6 +615,12 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>
> void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
> int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
> +
> +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);
> +
> struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct
> drm_atomic_state *state,
> struct
> drm_dp_mst_topology_mgr *mgr);
> int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
--
Cheers,
Lyude Paul
More information about the amd-gfx
mailing list