[Intel-gfx] [PATCH] drm/i915: Update the status of connector when receiving MST unplug event

Thu Aug 3 08:43:17 UTC 2017

On Wed, Aug 2, 2017 at 7:41 PM, Daniel Vetter <daniel at ffwll.ch> wrote:

> On Wed, Aug 02, 2017 at 01:49:18AM +0000, Pandiyan, Dhinakaran wrote:
> >
> > On Tue, 2017-08-01 at 16:51 +0800, Ethan Hsieh wrote:
> > > We do not update the status of connector when receiving MST unplug
> event.
> > > Call detect function to get latest status and then update status of
> connector.
> > >
> >
> > Cc'ing Daniel and Chris.
> >
> > Thanks for sending this to the list.
> >
> > The issue is connector ref count is not zero when
> > destroy_mst_connector() is called, which results in the connector not
> > being freed until the userspace shuts down the crtc tied to the
> > connector. But the kernel needs to update the connector status for the
> > user space to shut it down.
>
> Here's what's supposed to happen:
> 1. kernel sends out uevent
> 2. userspace does a full probe using GetConnector
> 3. kernel calls down into ->probe, updates connector->status
> 4. userspace notices the connector is disconnected, shuts down the
> CRTC/connector pipe
> 5. Last reference is dropped, connector disappears
> 6. Another round of uevent for the kernel removal (I think, not sure about
> this one).
>
> I suspect your userspace fails to do 2 if you need this patch.
> -Daniel
>
>
userspace receives uevent, and does a full probe using GetConnctor.

$ udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
KERNEL remove   /devices/pci0000:00/0000:00:02.0/drm/card0/card0-DP-4 (drm)
KERNEL change   /devices/pci0000:00/0000:00:02.0/drm/card0 (drm)
UDEV   remove   /devices/pci0000:00/0000:00:02.0/drm/card0/card0-DP-4 (drm)
UDEV   change   /devices/pci0000:00/0000:00:02.0/drm/card0 (drm)

kern.log:
[57.120672] [drm:drm_mode_getconnector [drm]] [CONNECTOR:74:DP-4] status:
connected
[57.120677] [drm:drm_mode_getconnector [drm]] out_resp->count_modes != 0

The status of DP-4 is connected, and fill_modes() isn't called.

---
If I update the status of connector in intel_dp_destroy_mst_connector,
fill_modes() will be executed.

kern.log:
[48.184569] [drm:drm_mode_getconnector [drm]] [CONNECTOR:74:DP-4] status:
disconnected
[48.184573] [drm:drm_mode_getconnector [drm]] fill_modes
[48.184577] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]]
[CONNECTOR:74:DP-4]
[48.184598] [drm:intel_dp_mst_detect [i915]] [CONNECTOR:74:DP-4] status:
disconnected
[48.184602] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]]
[CONNECTOR:74:DP-4] disconnected

> >
> >
> > > Before applying the patch:
> > > [313.665321] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> stat 0x00200000, dig 0x10101012, pins 0x00000020
> > > [313.665383] [drm:intel_hpd_irq_handler [i915]] digital hpd port B -
> long
> > > [313.665436] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt
> on PIN 5 - cnt: 0
> > > [313.665539] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B -
> long
> > > [313.944743] [drm:intel_dp_destroy_mst_connector [i915]]
> > > [313.944848] [drm:intel_dp_destroy_mst_connector [i915]]
> > >
> > > After applying the patch:
> > > [43.175798] [drm:intel_dp_destroy_mst_connector [i915]]
> [CONNECTOR:70:DP-4] status updated from connected to disconnected
> > > [43.175870] [drm:intel_dp_destroy_mst_connector [i915]]
> > > [43.177675] [drm:drm_helper_probe_single_connector_modes
> [drm_kms_helper]] [CONNECTOR:70:DP-4]
> > > [43.177679] [drm:drm_helper_probe_single_connector_modes
> [drm_kms_helper]] [CONNECTOR:70:DP-4] disconnected
> > >
> > > Signed-off-by: Ethan Hsieh <ethan.hsieh at canonical.com>
> >
> >
> > This patch needs a Fixes: tag as it does fix a fdo bug.
> >
> >
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_mst.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > index e4ea968..b02a9a8 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > @@ -492,6 +492,20 @@ static void intel_dp_destroy_mst_connector(struct
> drm_dp_mst_topology_mgr *mgr,
> > >  {
> > >     struct intel_connector *intel_connector =
> to_intel_connector(connector);
> > >     struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > > +   enum drm_connector_status old_status;
> > > +
> > > +   mutex_lock(&connector->dev->mode_config.mutex);
> > > +   old_status = connector->status;
> > > +   connector->status = connector->funcs->detect(connector, false);
> >
> > Isn't detect already done by this point? destroy_connector() should be
> > called after we know the display is disconnected.
>

intel_dp_mst_detect isn't called after unplugging the cable.
The display is disconnected, but the status of connector is still connected.
That is why I submitted the following patch. (The patch was rejected)
https://patchwork.freedesktop.org/patch/161729/

>
> > > +
> > > +   if (old_status != connector->status)
> > > +           DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s
> to %s\n",
> > > +                         connector->base.id,
> > > +                         connector->name,
> > > +                         drm_get_connector_status_name(old_status),
> > > +                         drm_get_connector_status_name(
> connector->status));
> > > +
> > > +   mutex_unlock(&connector->dev->mode_config.mutex);
> > >
> > >     drm_connector_unregister(connector);
> >
> > The connector is unregistered unconditionally here, so you might as well
> > set
> > connector->status = connector_status_disconnected;
> >
> > >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170803/20af0a4e/attachment-0001.html>