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:airl...@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-dp-mst-update-tile-property-after-edid-read.patch
Type: application/octet-stream
Size: 1160 bytes
Desc: 0001-drm-dp-mst-update-tile-property-after-edid-read.patch
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160223/33dcaa21/attachment.obj>

Reply via email to