[PATCH 3/5] drm/dp/mst: change MST detection scheme
Grodzovsky, Andrey
Andrey.Grodzovsky at amd.com
Thu May 12 22:41:21 UTC 2016
Hi Dave,
Have you had a chance to see if Mykola's latest patch addresses the issue you observed with tiled MST display ?
Thanks.
-----Original Message-----
From: Lysenko, Mykola
Sent: Monday, February 22, 2016 10:09 PM
To: Dave Airlie; Wentland, Harry
Cc: dri-devel
Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
Hi Dave,
Attached patch should fix the issue.
I have one question though: should we add drm_mode_connector_set_tile_property to the drm_get_displayid function, so it will be updated in case of regular DP displays as well?
As an example of such usage could be 5k displays with two tiles 2560x2880 that could not be driven on one MST connector because of bandwidth limitations, and are driven in SST mode.
Thanks,
Mykola
-----Original Message-----
From: Lysenko, Mykola
Sent: Thursday, February 18, 2016 12:53 PM
To: 'Dave Airlie' <airlied at gmail.com>; Wentland, Harry <Harry.Wentland at amd.com>
Cc: dri-devel <dri-devel at lists.freedesktop.org>
Subject: RE: [PATCH 3/5] drm/dp/mst: change MST detection scheme
Hi Dave,
it seems to be missed call to
drm_mode_connector_set_tile_property(port->connector);
here:
> + } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> + port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> + if (!port->cached_edid) {
> + port->cached_edid =
> + drm_get_edid(port->connector,
> + &port->aux.ddc);
drm_mode_connector_set_tile_property(port->connector);
> + }
I will test with tile display to see if it solves the issue.
Thanks,
Mykola
-----Original Message-----
From: Dave Airlie [mailto:airlied at gmail.com]
Sent: Wednesday, February 17, 2016 1:46 PM
To: Wentland, Harry <Harry.Wentland at amd.com>
Cc: dri-devel <dri-devel at lists.freedesktop.org>; Lysenko, Mykola <Mykola.Lysenko at amd.com>
Subject: Re: [PATCH 3/5] drm/dp/mst: change MST detection scheme
On 23 January 2016 at 08:07, Harry Wentland <harry.wentland at amd.com> wrote:
> From: Mykola Lysenko <Mykola.Lysenko at amd.com>
>
> 1. Get edid for all connected MST displays, not only on logical ports,
> in the same thread as MST topology detection is done:
> There are displays that have branches inside w/o logical ports.
> So in case another SST display connected downstream system can
> end-up in situation when 3 DOWN requests sent: two for
> ‘remote i2c read’ and one for ‘enum path resources’, making slots full.
>
> 2. Call notification callback in one place in the end of topology discovery/update:
> This is done to reduce number of events sent to userspace in case complex
> topology discovery is going, adding multiple number of
> connectors;
>
> 3. Remove notification callback call from short pulse interrupt processing function:
> This is done in order not to block interrupt processing function, in case any
> MST request will be made from it. Notification will be send from topology
> discovery/update work item.
I've had to pull this out, as I did some more indepth testing with
i915 and a Dell 30"
and this broke things.
The main thing it broke is setting the tiling property that userspace needs to see those dual-panel monitors as one.
you should be able to see xrandr --props if the tile property is correct if you test.
I'm also not sure about some other bits in this patch, so I probably need to dig a bit deeper into it.
Dave.
>
> Signed-off-by: Mykola Lysenko <Mykola.Lysenko at amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland at amd.com>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 37
> ++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 041597b7a7c6..052c20ca35ee 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1130,13 +1130,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
> drm_dp_put_port(port);
> goto out;
> }
> - if (port->port_num >= DP_MST_LOGICAL_PORT_0) {
> - port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
> - drm_mode_connector_set_tile_property(port->connector);
> - }
> +
> + drm_mode_connector_set_tile_property(port->connector);
> +
> (*mstb->mgr->cbs->register_connector)(port->connector);
> }
> -
> out:
> /* put reference to this port */
> drm_dp_put_port(port);
> @@ -1161,9 +1159,9 @@ static void drm_dp_update_port(struct drm_dp_mst_branch *mstb,
> port->ddps = conn_stat->displayport_device_plug_status;
>
> if (old_ddps != port->ddps) {
> + dowork = true;
> if (port->ddps) {
> drm_dp_check_port_guid(mstb, port);
> - dowork = true;
> } else {
> port->guid_valid = false;
> port->available_pbn = 0; @@ -1271,8 +1269,13
> @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
> if (port->input)
> continue;
>
> - if (!port->ddps)
> + if (!port->ddps) {
> + if (port->cached_edid) {
> + kfree(port->cached_edid);
> + port->cached_edid = NULL;
> + }
> continue;
> + }
>
> if (!port->available_pbn)
> drm_dp_send_enum_path_resources(mgr, mstb,
> port); @@ -1283,6 +1286,12 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
> drm_dp_check_and_send_link_address(mgr, mstb_child);
> drm_dp_put_mst_branch_device(mstb_child);
> }
> + } else if (port->pdt == DP_PEER_DEVICE_SST_SINK ||
> + port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV) {
> + if (!port->cached_edid) {
> + port->cached_edid =
> + drm_get_edid(port->connector, &port->aux.ddc);
> + }
> }
> }
> }
> @@ -1302,6 +1311,8 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
> drm_dp_check_and_send_link_address(mgr, mstb);
> drm_dp_put_mst_branch_device(mstb);
> }
> +
> + (*mgr->cbs->hotplug)(mgr);
> }
>
> static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> @@ -1558,7 +1569,6 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) {
> drm_dp_add_port(mstb, mgr->dev, &txmsg->reply.u.link_addr.ports[i]);
> }
> - (*mgr->cbs->hotplug)(mgr);
> }
> } else {
> mstb->link_address_sent = false; @@ -2232,8 +2242,6 @@
> static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr
> *mgr)
>
> drm_dp_update_port(mstb, &msg.u.conn_stat);
> DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number, msg.u.conn_stat.legacy_device_plug_status, msg.u.conn_stat.displayport_device_plug_status, msg.u.conn_stat.message_capability_status, msg.u.conn_stat.input_port, msg.u.conn_stat.peer_device_type);
> - (*mgr->cbs->hotplug)(mgr);
> -
> } else if (msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
> drm_dp_send_up_ack_reply(mgr, mgr->mst_primary, msg.req_type, seqno, false);
> if (!mstb)
> @@ -2320,10 +2328,6 @@ enum drm_connector_status
> drm_dp_mst_detect_port(struct drm_connector *connector
>
> case DP_PEER_DEVICE_SST_SINK:
> status = connector_status_connected;
> - /* for logical ports - cache the EDID */
> - if (port->port_num >= 8 && !port->cached_edid) {
> - port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> - }
> break;
> case DP_PEER_DEVICE_DP_LEGACY_CONV:
> if (port->ldps)
> @@ -2378,10 +2382,7 @@ struct edid *drm_dp_mst_get_edid(struct
> drm_connector *connector, struct drm_dp_
>
> if (port->cached_edid)
> edid = drm_edid_duplicate(port->cached_edid);
> - else {
> - edid = drm_get_edid(connector, &port->aux.ddc);
> - drm_mode_connector_set_tile_property(connector);
> - }
> +
> port->has_audio = drm_detect_monitor_audio(edid);
> drm_dp_put_port(port);
> return edid;
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list