Re: [PATCH] drm/dp: add module parameter for the dpcd access max retries

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, May 8, 2018 at 1:33 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Feng Tang (2018-05-08 20:03:15)
>>
>> On Mon, May 07, 2018 at 05:09:21PM +0200, Daniel Vetter wrote:
>> > On Mon, May 07, 2018 at 02:33:25PM +0100, Chris Wilson wrote:
>> > > Quoting Feng Tang (2018-05-07 22:26:34)
>> > > > Hi Chris,
>> > > >
>> > > > Thanks for the prompt review!
>> > > >
>> > > > On Mon, May 07, 2018 at 11:40:45AM +0100, Chris Wilson wrote:
>> > > > > Quoting Feng Tang (2018-05-07 11:36:09)
>> > > > > > To fulfil the Dell 4K monitor, the dpcd max retries has been bumped
>> > > > > > from 7 to 32, which may hurt the boot/init time for some platforms,
>> > > > > > as the 32 retries may take hundreds of ms.
>> > > > >
>> > > > > If we need that many retries, so be it. No modparam, the driver just has
>> > > > > to work.
>> > > >
>> > > > I understand your point. The retry numer was originally 7, and worked
>> > > > fine untill the Dell 4K monitor which changes to 32.  According to my test,
>> > > > each retry will take about 8ms on the A3960 based NUC.
>> > > >
>> > > > One of our product need to boot up within a given time limit, this
>> > > > 32 retries will take about 1/3 of the budget (about 270ms), that's
>> > > > why I would try to make it a parameter.
>> > >
>> > > The essence is that probing whether a monitor is connected should not be
>> > > blocking boot. If an async probe tries and fails to find a monitor,
>> > > fine - no one will notice. If it does take 270ms to find a monitor, it
>> > > turns on 200ms after userspace kicks in, just like any other hotplug.
>> >
>> > Yeah, the fix here is to get the probing out of the driver load path, not
>> > to break the driver for some funky monitors. And definitely not using a
>> > modparam.
>>
>> Hi Chris and Daniel,
>>
>> After reading the i915 modeset init code, I did some experiments:
>> 1. make the intel_modeset_init() function async (here async means
>>    creating a async func wrapper and call async_schedul() for it)
>> 2. make the intel_setup_outpus()+intel_modeset_setup_hw_state() async
>
> You could just set i915_pci_driver.driver.prove_type =
> PROBE_PREFER_ASYNCHRONOUS for the same result, or set i915.async_probe=1
> for testing.
>
> However, if it's blocking on async_synchronize_full(), that will be no
> improvement. So if it is only an existing async_sychronize_full()
> causing the fbdev config to be waited on before userspace, we need to
> stop using the async mechanism and just use an ordinary worker and
> manual flushing. If it's the fbdev probing blocking you, why are you
> using fbdev?

Well if it's edp probing, then atm we do need to block since we have
no support for panel hotplugging. And userspace generally no
expectations that built-in panels come&go. async_synchronize_full
making our fbdev code less async than desired is kinda a different
story I think here. First step would be trying to figure out why we
even bother with edp probing on this platform, when the thing isn't
there. Sounds like broken VBT.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx