[PATCH 3/5] drm/dp/mst: change MST detection scheme

Lysenko, Mykola Mykola.Lysenko at amd.com
Thu Feb 18 04:53:05 UTC 2016


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