[PATCH 3/5] drm/dp/mst: change MST detection scheme
Dave Airlie
airlied at gmail.com
Wed Jun 15 03:49:58 UTC 2016
Excuse me for top posting.
So I finally got back to this patch, still don't like it.
Apart from the doing 3 things in once which is just annoying, current
userspace for tiled monitors
relies on the behaviour that the cached edids are retrieved before the
connector is show to userspace.
This is so that for tiled monitors both connectors are shown with
tiling properties so userspace can pick the correct monitor size.
Now I realise this isn't great, since at least for the two cable 5K
monitors you won't ever plug them in at once.
So we should probably discuss more what userspace should do in the
presence of a tiled monitor where it can only see one tile,
If we go ahead with something like this, things will look kinda ugly
as you plug in a 4k monitor and it'll try and set a mode
on the first tile when it gets the hotplug (or it could be racing and
notice the connectors before the hotplug). So it then
sets the 1920x1080 mode on one connector and gets another hotplug to
set the tiled mode up.
Dave.
On 13 May 2016 at 08:41, Grodzovsky, Andrey <Andrey.Grodzovsky at amd.com> wrote:
> 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