<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 2, 2017 at 7:41 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On Wed, Aug 02, 2017 at 01:49:18AM +0000, Pandiyan, Dhinakaran wrote:<br>
><br>
> On Tue, 2017-08-01 at 16:51 +0800, Ethan Hsieh wrote:<br>
> > We do not update the status of connector when receiving MST unplug event.<br>
> > Call detect function to get latest status and then update status of connector.<br>
> ><br>
><br>
> Cc'ing Daniel and Chris.<br>
><br>
> Thanks for sending this to the list.<br>
><br>
> The issue is connector ref count is not zero when<br>
> destroy_mst_connector() is called, which results in the connector not<br>
> being freed until the userspace shuts down the crtc tied to the<br>
> connector. But the kernel needs to update the connector status for the<br>
> user space to shut it down.<br>
<br>
</span>Here's what's supposed to happen:<br>
1. kernel sends out uevent<br>
2. userspace does a full probe using GetConnector<br>
3. kernel calls down into ->probe, updates connector->status<br>
4. userspace notices the connector is disconnected, shuts down the<br>
CRTC/connector pipe<br>
5. Last reference is dropped, connector disappears<br>
6. Another round of uevent for the kernel removal (I think, not sure about<br>
this one).<br>
<br>
I suspect your userspace fails to do 2 if you need this patch.<br>
-Daniel<br>
<div class="gmail-HOEnZb"><div class="gmail-h5"><br></div></div></blockquote><div><br></div><div>userspace receives uevent, and does a full probe using GetConnctor.</div><div><br></div><div><div>$ udevadm monitor</div><div>monitor will print the received events for:</div><div>UDEV - the event which udev sends out after rule processing</div><div>KERNEL - the kernel uevent</div><div>KERNEL remove   /devices/pci0000:00/0000:00:02.0/drm/card0/card0-DP-4 (drm)</div><div>KERNEL change   /devices/pci0000:00/0000:00:02.0/drm/card0 (drm)</div><div>UDEV   remove   /devices/pci0000:00/0000:00:02.0/drm/card0/card0-DP-4 (drm)</div><div>UDEV   change   /devices/pci0000:00/0000:00:02.0/drm/card0 (drm)</div><div><br></div><div>kern.log:</div><div>[57.120672] [drm:drm_mode_getconnector [drm]] [CONNECTOR:74:DP-4] status: connected</div><div>[57.120677] [drm:drm_mode_getconnector [drm]] out_resp->count_modes != 0</div><div><br></div><div>The status of DP-4 is connected, and fill_modes() isn't called.<br></div><div><br></div><div>---</div><div>If I update the status of connector in intel_dp_destroy_mst_<wbr>connector, fill_modes() will be executed.</div><div><br></div><div>kern.log:</div><div>[48.184569] [drm:drm_mode_getconnector [drm]] [CONNECTOR:74:DP-4] status: disconnected</div><div>[48.184573] [drm:drm_mode_getconnector [drm]] fill_modes</div><div>[48.184577] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:74:DP-4]</div><div>[48.184598] [drm:intel_dp_mst_detect [i915]] [CONNECTOR:74:DP-4] status: disconnected</div><div>[48.184602] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:74:DP-4] disconnected</div></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">
><br>
><br>
> > Before applying the patch:<br>
> > [313.665321] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x10101012, pins 0x00000020<br>
> > [313.665383] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long<br>
> > [313.665436] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 5 - cnt: 0<br>
> > [313.665539] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long<br>
> > [313.944743] [drm:intel_dp_destroy_mst_<wbr>connector [i915]]<br>
> > [313.944848] [drm:intel_dp_destroy_mst_<wbr>connector [i915]]<br>
> ><br>
> > After applying the patch:<br>
> > [43.175798] [drm:intel_dp_destroy_mst_<wbr>connector [i915]] [CONNECTOR:70:DP-4] status updated from connected to disconnected<br>
> > [43.175870] [drm:intel_dp_destroy_mst_<wbr>connector [i915]]<br>
> > [43.177675] [drm:drm_helper_probe_single_<wbr>connector_modes [drm_kms_helper]] [CONNECTOR:70:DP-4]<br>
> > [43.177679] [drm:drm_helper_probe_single_<wbr>connector_modes [drm_kms_helper]] [CONNECTOR:70:DP-4] disconnected<br>
> ><br>
> > Signed-off-by: Ethan Hsieh <<a href="mailto:ethan.hsieh@canonical.com">ethan.hsieh@canonical.com</a>><br>
><br>
><br>
> This patch needs a Fixes: tag as it does fix a fdo bug.<br>
><br>
><br>
> > ---<br>
> >  drivers/gpu/drm/i915/intel_dp_<wbr>mst.c | 14 ++++++++++++++<br>
> >  1 file changed, 14 insertions(+)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/i915/intel_<wbr>dp_mst.c b/drivers/gpu/drm/i915/intel_<wbr>dp_mst.c<br>
> > index e4ea968..b02a9a8 100644<br>
> > --- a/drivers/gpu/drm/i915/intel_<wbr>dp_mst.c<br>
> > +++ b/drivers/gpu/drm/i915/intel_<wbr>dp_mst.c<br>
> > @@ -492,6 +492,20 @@ static void intel_dp_destroy_mst_<wbr>connector(struct drm_dp_mst_topology_mgr *mgr,<br>
> >  {<br>
> >     struct intel_connector *intel_connector = to_intel_connector(connector);<br>
> >     struct drm_i915_private *dev_priv = to_i915(connector->dev);<br>
> > +   enum drm_connector_status old_status;<br>
> > +<br>
> > +   mutex_lock(&connector->dev-><wbr>mode_config.mutex);<br>
> > +   old_status = connector->status;<br>
> > +   connector->status = connector->funcs->detect(<wbr>connector, false);<br>
><br>
> Isn't detect already done by this point? destroy_connector() should be<br>
> called after we know the display is disconnected.</div></div></blockquote><div><br></div><div>intel_dp_mst_detect isn't called after unplugging the cable.</div><div>The display is disconnected, but the status of connector is still connected.<br></div><div>That is why I submitted the following patch. (The patch was rejected)</div><div><a href="https://patchwork.freedesktop.org/patch/161729/">https://patchwork.freedesktop.org/patch/161729/</a><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">
><br>
> > +<br>
> > +   if (old_status != connector->status)<br>
> > +           DRM_DEBUG_KMS("[CONNECTOR:%d:%<wbr>s] status updated from %s to %s\n",<br>
> > +                         connector-><a href="http://base.id" rel="noreferrer" target="_blank">base.id</a>,<br>
> > +                         connector->name,<br>
> > +                         drm_get_connector_status_name(<wbr>old_status),<br>
> > +                         drm_get_connector_status_name(<wbr>connector->status));<br>
> > +<br>
> > +   mutex_unlock(&connector->dev-><wbr>mode_config.mutex);<br>
> ><br>
> >     drm_connector_unregister(<wbr>connector);<br>
><br>
> The connector is unregistered unconditionally here, so you might as well<br>
> set<br>
> connector->status = connector_status_disconnected;<br>
><br>
> ><br>
<br>
</div></div><span class="gmail-HOEnZb"><font color="#888888">--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="http://blog.ffwll.ch" rel="noreferrer" target="_blank">http://blog.ffwll.ch</a><br>
</font></span></blockquote></div><br></div></div>