Messages in this thread | | | Date | Fri, 15 Apr 2022 17:06:48 -0700 | Subject | Re: [PATCH v2] drm/msm/dp: enhance both connect and disconnect pending_timeout handle | From | Kuogee Hsieh <> |
| |
On 4/14/2022 4:34 PM, Dmitry Baryshkov wrote: > On 15/04/2022 01:06, Kuogee Hsieh wrote: >> >> On 4/8/2022 4:59 PM, Dmitry Baryshkov wrote: >>> On Fri, 8 Apr 2022 at 23:30, Kuogee Hsieh <quic_khsieh@quicinc.com> >>> wrote: >>>> >>>> On 4/8/2022 5:27 AM, Dmitry Baryshkov wrote: >>>>> On 07/04/2022 00:28, Kuogee Hsieh wrote: >>>>>> dp_hpd_plug_handle() is responsible for setting up main link and >>>>>> send >>>>>> uevent to notify user space framework to start video stream. >>>>>> Similarly, >>>>>> dp_hdp_unplug_handle is responsible to send uevent to notify user >>>>>> space >>>>>> framework to stop video stream and then tear down main link. >>>>>> However there are rare cases, such as in the middle of system >>>>>> suspending, >>>>>> that uevent could not be delivered to user space framework. >>>>>> Therefore >>>>>> some kind of recover mechanism armed by timer need to be in place >>>>>> in the >>>>>> case of user space framework does not respond to uevent. >>>>> Hmm, how does userpsace 'respond' to the uevent? The driver should >>>>> send hotplug notifications to userspace, but it must not expect any >>>>> particular reaction. The userspace might be as simple, as fbdev >>>>> emulation, but the driver still should function correctly. >>>> yes, driver is function correctly by setting up main link. but it does >>>> not know which resolution to display. >>>> >>>> It send hotplug notification through uevent to framework after main >>>> link >>>> is ready. >>>> >>>> Framework is responsible to set up MDP timing engine to start >>>> video stream. >>>> >>>> >>>> However it does not know which >>> It's of no concern to the driver. It is completely the userspace >>> problem. After resuming, it should reread available video output >>> properties. The display could have been changed while the system is >>> suspended. >>> From your description I still do not understand why you need the >>> 'recovery' mechanism. >> >> I mean "recovery" means dp driver may leave stay at link ready >> mistakenly after dongle unplugged due to missing framework action to >> tear down main link so that next dongle plug in will not work. >> >> Currently it was armed with timeout function and it will fired if >> framework did not call msm_dp_display_disable() after 5 second. > > framework = userspace? > > Is my understanding correct? Currently DP driver sends a notification > to userspace that DP is unplugged, waits for userspace to disable DP > output, and only after that it will shutdown the link. Is this a > correct description? Yes, you are correct, connection need to be tear down from top to bottom. > > If so, then yes, your change is correct. I mean that the kernel > shouldn't wait for clients to stop using video pipeline if the sink > gets unplugged. Instead it should tear the video stream down and let > the DRM client cope with that. > > I'm not sure how should the driver react if the client doesn't disable > the output, but then the sink gets reattached?
I do not know that either.
But it should not happen as long as framework response to uevent.
> And a bit more interesting question: how should the driver behave if > the new sink is not compatible with the existing video stream.
When dongle plug/replug in, driver will always read sink's edid.
Therefore resolution database should be updated accordingly.
> >> >> Anyway, we think this approach is not good. therefore it is replaced >> with below patch. >> >> drm/msm/dp: tear down main link at unplug handle immediately > >
| |