diff mbox

drm/dp: add module parameter for the dpcd access max retries

Message ID 1525689369-27124-1-git-send-email-feng.tang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Feng Tang May 7, 2018, 10:36 a.m. UTC
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.

This patch makes no functional change, but make the max retries
adjustable, so that concerned users/platforms can have an option
to short the wait time.

On a Intel Atom Processor A3960 based NUC, the i915_init() time could
be reduced from 450ms to 200ms.

retries = 3:
	[    0.457806] calling  i915_init+0x0/0x51 @ 1
	[    0.662307] initcall i915_init+0x0/0x51 returned 0 after 199694 usecs

retries = 32:
	[    0.465818] calling  i915_init+0x0/0x51 @ 1
	[    0.925831] initcall i915_init+0x0/0x51 returned 0 after 449219 usecs

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Comments

Chris Wilson May 7, 2018, 10:40 a.m. UTC | #1
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.
 
> This patch makes no functional change, but make the max retries
> adjustable, so that concerned users/platforms can have an option
> to short the wait time.
> 
> On a Intel Atom Processor A3960 based NUC, the i915_init() time could
> be reduced from 450ms to 200ms.
> 
> retries = 3:
>         [    0.457806] calling  i915_init+0x0/0x51 @ 1
>         [    0.662307] initcall i915_init+0x0/0x51 returned 0 after 199694 usecs
> 
> retries = 32:
>         [    0.465818] calling  i915_init+0x0/0x51 @ 1
>         [    0.925831] initcall i915_init+0x0/0x51 returned 0 after 449219 usecs

Why is this synchronous anyway?
-Chris
Chris Wilson May 7, 2018, 1:33 p.m. UTC | #2
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.
-Chris
Daniel Vetter May 7, 2018, 3:09 p.m. UTC | #3
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.
-Daniel
Feng Tang May 7, 2018, 9:26 p.m. UTC | #4
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.

>  
> > This patch makes no functional change, but make the max retries
> > adjustable, so that concerned users/platforms can have an option
> > to short the wait time.
> > 
> > On a Intel Atom Processor A3960 based NUC, the i915_init() time could
> > be reduced from 450ms to 200ms.
> > 
> > retries = 3:
> >         [    0.457806] calling  i915_init+0x0/0x51 @ 1
> >         [    0.662307] initcall i915_init+0x0/0x51 returned 0 after 199694 usecs
> > 
> > retries = 32:
> >         [    0.465818] calling  i915_init+0x0/0x51 @ 1
> >         [    0.925831] initcall i915_init+0x0/0x51 returned 0 after 449219 usecs
> 
> Why is this synchronous anyway?

I don't know the reason, maybe some of kernel components depends on the GFX module.
But even we can put it to async, it is still the longest init one on our platform.

Thanks,
Feng
Feng Tang May 7, 2018, 11:19 p.m. UTC | #5
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.

Thank you both for the suggestion! 

Will try to make the probing async first, though I'm not familiar with the
whole drm yet :)

- Feng
Chris Wilson May 8, 2018, 11:33 a.m. UTC | #6
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?
-Chris
Daniel Vetter May 8, 2018, noon UTC | #7
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
Feng Tang May 8, 2018, 7:03 p.m. UTC | #8
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 

But both of them will trigger kernel panic (log msg pasted in the end),
did I made some mistakes, or maybe the i915 codes following these functions
has dependency over them?

IIUC the dpcd access first happens in
    i915_driver_load --> i915_load_modeset_init --> intel_modeset_init
    --> intel_setup_outputs --> intel_ddi_init --> intel_edp_init_connector
    --> intel_edp_init_dpcd (to check if DPCD exist)

Should we postpone it to later phase or even after user space kick in?

Thanks,
Feng

---
Error msg for my async experiment:

[    0.715706] No backend configured for hyper_dmabuf in kernel config
[    0.716079] Hyper_dmabuf: no backend found
[    0.736361] intel_powerclamp: CPU does not support MWAIT
[    0.737643] [drm:wait_panel_status] *ERROR* PPS state mismatch
[    0.741381] genirq: Setting trigger mode 3 for irq 127 failed (intel_gpio_irq_type+0x0/0x110)
[    0.743244] dmi-sysfs: dmi entry is absent.
[    0.765116] [edp_panel_vdd_on()]: exit
[    0.765360] BUG: unable to handle kernel NULL pointer dereference at           (null)
[    0.765809] IP:           (null)
[    0.766005] PGD 0 P4D 0 
[    0.766168] Oops: 0010 [#1] PREEMPT SMP
[    0.766401] Modules linked in:
[    0.766592] CPU: 0 PID: 28 Comm: kworker/u8:1 Tainted: G     U  W       4.14.39-sos+ #26
[    0.767075] Workqueue: events_unbound async_run_entry_fn
[    0.767392] task: ffff88027433c240 task.stack: ffff880274340000
[    0.767743] RIP: 0010:          (null)
[    0.767969] RSP: 0000:ffff880274343ab8 EFLAGS: 00010246
[    0.768281] RAX: 0000000000000000 RBX: 000000002d4003ff RCX: 0000000000000001
[    0.768701] RDX: 0000000080000000 RSI: 0000000000000000 RDI: ffff880272f21100
[    0.769121] RBP: 0000000000000000 R08: 000000002d4003ff R09: 0000000000000001
[    0.769541] R10: ffff880274343a60 R11: ffffffff82e7fe0d R12: 0000000000000001
[    0.769961] R13: ffff880273038000 R14: 0000000000000004 R15: ffff880272f21100
[    0.770383] FS:  0000000000000000(0000) GS:ffff88027dc00000(0000) knlGS:0000000000000000
[    0.770858] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.771199] CR2: 0000000000000000 CR3: 0000000002613000 CR4: 00000000003426f0
[    0.771619] Call Trace:
[    0.771779]  ? intel_dp_aux_ch+0x1a7/0x770
[    0.772031]  ? remove_wait_queue+0x60/0x60
[    0.772281]  ? intel_dp_aux_transfer+0xa6/0x200
[    0.772556]  ? drm_dp_dpcd_access+0x9d/0x150
[    0.772815]  ? drm_dp_dpcd_read+0x2c/0x60
[    0.773059]  ? drm_dp_read_desc+0x43/0xf0
[    0.773303]  ? intel_dp_detect+0x346/0x6a0
[    0.773554]  ? drm_helper_probe_single_connector_modes+0xcd/0x6b0
[    0.773920]  ? _raw_spin_unlock+0x14/0x30
[    0.774165]  ? vt_console_print+0x22a/0x3d0
[    0.774420]  ? preempt_count_add+0x56/0xa0
[    0.774669]  ? _raw_spin_lock_irqsave+0x32/0x40
[    0.774944]  ? drm_setup_crtcs+0x143/0x9e0
[    0.775195]  ? __drm_fb_helper_initial_config_and_unlock+0x3f/0x410
[    0.775567]  ? mutex_lock+0x1c/0x40
[    0.775783]  ? intel_fbdev_initial_config+0x14/0x30
[    0.776076]  ? async_run_entry_fn+0x39/0x160
[    0.776335]  ? process_one_work+0x14a/0x3c0
[    0.776588]  ? worker_thread+0x4d/0x3e0
[    0.776823]  ? kthread+0x10a/0x140
[    0.777031]  ? process_one_work+0x3c0/0x3c0
[    0.777284]  ? kthread_create_on_node+0x40/0x40
[    0.777557]  ? ret_from_fork+0x3a/0x50
[    0.777785] Code:  Bad RIP value.
[    0.777995] RIP:           (null) RSP: ffff880274343ab8
[    0.778305] CR2: 0000000000000000
[    0.778508] ---[ end trace f4e157ae338a4060 ]---
[    0.778786] Kernel panic - not syncing: Fatal exception
[    0.779160] Kernel Offset: disabled
[    0.779376] Rebooting in 10 seconds..
Jani Nikula May 8, 2018, 7:30 p.m. UTC | #9
On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
>  >> > > > 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.
>
> Hi Daniel,
>
> Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
> based NUC. but I don't have the knowledge to tell if the VBT is broken :)

Please run current upstream drm-tip when you're suggesting changes to
upstream code. Looks like you're running at most v4.14. This can't be
emphasized enough. We can't and won't merge the changes unless they make
sense with current code.

As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt.

BR,
Jani.

>
> [    0.545231] [drm:intel_bios_init] Set default to SSC at 100000 kHz
> [    0.545237] [drm:intel_bios_init] VBT signature "$VBT BROXTON        ", BDB version 207
> [    0.545241] [drm:intel_bios_init] BDB_GENERAL_FEATURES int_tv_support 0 int_crt_support 0 lvds_use_ssc 0 lvds_ssc_freq 120000 display_clock_mode 1 fdi_rx_polarity_inverted 0
> [    0.545245] [drm:intel_bios_init] crt_ddc_bus_pin: 2
> [    0.545255] [drm:intel_opregion_get_panel_type] Failed to get panel details from OpRegion (-19)
> [    0.545257] [drm:intel_bios_init] Panel type: 0 (VBT)
> [    0.545260] [drm:intel_bios_init] DRRS supported mode is seamless
> [    0.545275] [drm:intel_bios_init] Found panel mode in BIOS VBT tables:
> [    0.545281] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 0 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x8 0xa
> [    0.545292] [drm:intel_bios_init] VBT backlight PWM modulation frequency 200 Hz, active high, min brightness 0, level 180, controller 1
> [    0.545301] [drm:intel_bios_init] Unsupported child device size for SDVO mapping.
> [    0.545305] [drm:intel_bios_init] Expected child device config size for VBT version 207 not known; assuming 38
> [    0.545323] [drm:intel_bios_init] DRRS State Enabled:1
> [    0.545334] [drm:intel_bios_init] Port A VBT info: DP:1 HDMI:0 DVI:0 EDP:1 CRT:0
> [    0.545338] [drm:intel_bios_init] VBT HDMI level shift for port A: 0
> [    0.545341] [drm:intel_bios_init] Port B VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0
> [    0.545344] [drm:intel_bios_init] VBT HDMI level shift for port B: 8
> [    0.545347] [drm:intel_bios_init] Port C VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0
> [    0.545350] [drm:intel_bios_init] VBT HDMI level shift for port C: 8
> [    0.546076] [drm:gen9_set_dc_state] Setting DC state from 00 to 00
> [    0.546091] [drm:intel_power_well_enable] enabling power well 1
> [    0.546126] [drm:gvt_service_thread] gvt: core: service thread start
> [    0.546174] [drm:intel_update_cdclk] Current CD clock rate: 19200 kHz, VCO: 0 kHz, ref: 19200 kHz
> [    0.546176] [drm:bxt_init_cdclk] Sanitizing cdclk programmed by pre-os
> [    0.546339] [drm:intel_update_cdclk] Current CD clock rate: 144000 kHz, VCO: 1152000 kHz, ref: 19200 kHz
> [    0.546357] [drm:intel_power_well_enable] enabling always-on
> [    0.546360] [drm:intel_power_well_enable] enabling DC off
> [    0.546364] [drm:gen9_set_dc_state] Setting DC state from 00 to 00
> [    0.546372] [drm:intel_power_well_enable] enabling power well 2
> [    0.546386] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
> [    0.546396] [drm:intel_power_well_enable] enabling dpio-common-a
> [    0.546488] [drm:intel_power_well_enable] enabling dpio-common-bc
> [    0.546844] [drm:intel_csr_ucode_init] Loading i915/bxt_dmc_ver1_07.bin
> [    0.546854] [drm:drm_irq_install] irq=124
> [    0.546906] i915 0000:00:02.0: Direct firmware load for i915/bxt_dmc_ver1_07.bin failed with error -2
> [    0.546909] i915 0000:00:02.0: Falling back to user helper
> [    0.547098] [drm:__bxt_hpd_detection_setup] Invert bit setting: hp_ctl:18001818 hp_port:38
> [    0.547425] [drm:intel_fbc_init] Sanitized enable_fbc value: 1
> [    0.547445] [drm:intel_print_wm_latency] Gen9 Plane WM0 latency 7 (7.0 usec)
> [    0.547447] [drm:intel_print_wm_latency] Gen9 Plane WM1 latency 7 (7.0 usec)
> [    0.547450] [drm:intel_print_wm_latency] Gen9 Plane WM2 latency 8 (8.0 usec)
> [    0.547453] [drm:intel_print_wm_latency] Gen9 Plane WM3 latency 22 (22.0 usec)
> [    0.547455] [drm:intel_print_wm_latency] Gen9 Plane WM4 latency 22 (22.0 usec)
> [    0.547458] [drm:intel_print_wm_latency] Gen9 Plane WM5 latency 22 (22.0 usec)
> [    0.547460] [drm:intel_print_wm_latency] Gen9 Plane WM6 latency 22 (22.0 usec)
> [    0.547462] [drm:intel_print_wm_latency] Gen9 Plane WM7 latency 22 (22.0 usec)
> [    0.547466] [drm:intel_modeset_init] 3 display pipes available.
> [    0.548182] [drm:intel_update_cdclk] Current CD clock rate: 144000 kHz, VCO: 1152000 kHz, ref: 19200 kHz
> [    0.548393] [drm:intel_update_max_cdclk] Max CD clock rate: 624000 kHz
> [    0.548395] [drm:intel_modeset_init] Max dotclock rate: 624000 kHz
> [    0.549124] [intel_setup_outputs()]: enter
> [    0.549522] [drm:intel_ddi_init] BXT BIOS forgot to set DDI_A_4_LANES for port A; fixing
> [    0.549531] [drm:intel_dp_init_connector] Adding eDP connector on port A
> [    0.549538] [drm:intel_dp_init_connector] using AUX A for port A (VBT)
> [    0.549552] [drm:intel_pps_dump_state] cur t1_t3 0 t8 0 t9 0 t10 0 t11_t12 0
> [    0.549556] [drm:intel_pps_dump_state] vbt t1_t3 2000 t8 10 t9 2000 t10 500 t11_t12 6000
> [    0.549560] [drm:intel_dp_init_panel_power_sequencer.part.19] panel power up delay 200, power down delay 50, power cycle delay 600
> [    0.549563] [drm:intel_dp_init_panel_power_sequencer.part.19] backlight on delay 1, off delay 200
> [    0.549569] [drm:intel_dp_init_panel_power_sequencer_registers] panel power sequencer register settings: PP_ON 0x7d00001, PP_OFF 0x1f40001, PP_DIV 0x60
> [    0.549577] [drm:edp_panel_vdd_on] Turning eDP port A VDD on
> [    0.549580] [drm:wait_panel_power_cycle] Wait for panel power cycle
> [    0.601112] [drm:wait_panel_status] mask b800000f value 00000000 status 00000000 control 00000062
> [    0.601116] [drm:wait_panel_status] Wait complete
> [    0.601121] [drm:edp_panel_vdd_on] PP_STATUS: 0x00000000 PP_CONTROL: 0x0000006a
> [    0.601125] [drm:edp_panel_vdd_on] eDP port A panel power wasn't enabled
> [    0.636720] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.645328] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.653926] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.662531] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.671132] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.679767] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.688366] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.696969] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.705572] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.714173] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.722770] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.731378] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.739983] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.748590] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.757210] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.765808] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.774422] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.783039] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.791642] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.800252] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.808859] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.817481] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.826086] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.834722] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.843335] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.851944] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.860552] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.869160] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.877767] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.886429] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.895045] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.903658] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
> [    0.903665] [drm:drm_dp_dpcd_access] Too many retries, giving up. First error: -110
> [    0.903666] [drm] failed to retrieve link info, disabling eDP
> [    0.903671] [drm:edp_panel_vdd_off_sync] Turning eDP port A VDD off
> [    0.903676] [drm:edp_panel_vdd_off_sync] PP_STATUS: 0x00000000 PP_CONTROL: 0x00000062
> [    0.903695] [drm:intel_hdmi_init_connector] Adding HDMI connector on port B
> [    0.903702] [drm:intel_hdmi_init_connector] Using DDC pin 0x1 for port B (VBT)
> [    0.903720] [drm:intel_hdmi_init_connector] Adding HDMI connector on port C
> [    0.903724] [drm:intel_hdmi_init_connector] Using DDC pin 0x2 for port C (VBT)
> [    0.903728] [drm:intel_dsi_init] 
> [    0.903733] [drm:acrngt_dom0_ready] gvt: core: acrngt: Dom 0 ready to accept Dom U guests
>
> Thanks,
> Feng
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Feng Tang May 8, 2018, 11:08 p.m. UTC | #10
>> > > > 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.

Hi Daniel,

Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
based NUC. but I don't have the knowledge to tell if the VBT is broken :)

[    0.545231] [drm:intel_bios_init] Set default to SSC at 100000 kHz
[    0.545237] [drm:intel_bios_init] VBT signature "$VBT BROXTON        ", BDB version 207
[    0.545241] [drm:intel_bios_init] BDB_GENERAL_FEATURES int_tv_support 0 int_crt_support 0 lvds_use_ssc 0 lvds_ssc_freq 120000 display_clock_mode 1 fdi_rx_polarity_inverted 0
[    0.545245] [drm:intel_bios_init] crt_ddc_bus_pin: 2
[    0.545255] [drm:intel_opregion_get_panel_type] Failed to get panel details from OpRegion (-19)
[    0.545257] [drm:intel_bios_init] Panel type: 0 (VBT)
[    0.545260] [drm:intel_bios_init] DRRS supported mode is seamless
[    0.545275] [drm:intel_bios_init] Found panel mode in BIOS VBT tables:
[    0.545281] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 0 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x8 0xa
[    0.545292] [drm:intel_bios_init] VBT backlight PWM modulation frequency 200 Hz, active high, min brightness 0, level 180, controller 1
[    0.545301] [drm:intel_bios_init] Unsupported child device size for SDVO mapping.
[    0.545305] [drm:intel_bios_init] Expected child device config size for VBT version 207 not known; assuming 38
[    0.545323] [drm:intel_bios_init] DRRS State Enabled:1
[    0.545334] [drm:intel_bios_init] Port A VBT info: DP:1 HDMI:0 DVI:0 EDP:1 CRT:0
[    0.545338] [drm:intel_bios_init] VBT HDMI level shift for port A: 0
[    0.545341] [drm:intel_bios_init] Port B VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0
[    0.545344] [drm:intel_bios_init] VBT HDMI level shift for port B: 8
[    0.545347] [drm:intel_bios_init] Port C VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0
[    0.545350] [drm:intel_bios_init] VBT HDMI level shift for port C: 8
[    0.546076] [drm:gen9_set_dc_state] Setting DC state from 00 to 00
[    0.546091] [drm:intel_power_well_enable] enabling power well 1
[    0.546126] [drm:gvt_service_thread] gvt: core: service thread start
[    0.546174] [drm:intel_update_cdclk] Current CD clock rate: 19200 kHz, VCO: 0 kHz, ref: 19200 kHz
[    0.546176] [drm:bxt_init_cdclk] Sanitizing cdclk programmed by pre-os
[    0.546339] [drm:intel_update_cdclk] Current CD clock rate: 144000 kHz, VCO: 1152000 kHz, ref: 19200 kHz
[    0.546357] [drm:intel_power_well_enable] enabling always-on
[    0.546360] [drm:intel_power_well_enable] enabling DC off
[    0.546364] [drm:gen9_set_dc_state] Setting DC state from 00 to 00
[    0.546372] [drm:intel_power_well_enable] enabling power well 2
[    0.546386] i915 0000:00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem
[    0.546396] [drm:intel_power_well_enable] enabling dpio-common-a
[    0.546488] [drm:intel_power_well_enable] enabling dpio-common-bc
[    0.546844] [drm:intel_csr_ucode_init] Loading i915/bxt_dmc_ver1_07.bin
[    0.546854] [drm:drm_irq_install] irq=124
[    0.546906] i915 0000:00:02.0: Direct firmware load for i915/bxt_dmc_ver1_07.bin failed with error -2
[    0.546909] i915 0000:00:02.0: Falling back to user helper
[    0.547098] [drm:__bxt_hpd_detection_setup] Invert bit setting: hp_ctl:18001818 hp_port:38
[    0.547425] [drm:intel_fbc_init] Sanitized enable_fbc value: 1
[    0.547445] [drm:intel_print_wm_latency] Gen9 Plane WM0 latency 7 (7.0 usec)
[    0.547447] [drm:intel_print_wm_latency] Gen9 Plane WM1 latency 7 (7.0 usec)
[    0.547450] [drm:intel_print_wm_latency] Gen9 Plane WM2 latency 8 (8.0 usec)
[    0.547453] [drm:intel_print_wm_latency] Gen9 Plane WM3 latency 22 (22.0 usec)
[    0.547455] [drm:intel_print_wm_latency] Gen9 Plane WM4 latency 22 (22.0 usec)
[    0.547458] [drm:intel_print_wm_latency] Gen9 Plane WM5 latency 22 (22.0 usec)
[    0.547460] [drm:intel_print_wm_latency] Gen9 Plane WM6 latency 22 (22.0 usec)
[    0.547462] [drm:intel_print_wm_latency] Gen9 Plane WM7 latency 22 (22.0 usec)
[    0.547466] [drm:intel_modeset_init] 3 display pipes available.
[    0.548182] [drm:intel_update_cdclk] Current CD clock rate: 144000 kHz, VCO: 1152000 kHz, ref: 19200 kHz
[    0.548393] [drm:intel_update_max_cdclk] Max CD clock rate: 624000 kHz
[    0.548395] [drm:intel_modeset_init] Max dotclock rate: 624000 kHz
[    0.549124] [intel_setup_outputs()]: enter
[    0.549522] [drm:intel_ddi_init] BXT BIOS forgot to set DDI_A_4_LANES for port A; fixing
[    0.549531] [drm:intel_dp_init_connector] Adding eDP connector on port A
[    0.549538] [drm:intel_dp_init_connector] using AUX A for port A (VBT)
[    0.549552] [drm:intel_pps_dump_state] cur t1_t3 0 t8 0 t9 0 t10 0 t11_t12 0
[    0.549556] [drm:intel_pps_dump_state] vbt t1_t3 2000 t8 10 t9 2000 t10 500 t11_t12 6000
[    0.549560] [drm:intel_dp_init_panel_power_sequencer.part.19] panel power up delay 200, power down delay 50, power cycle delay 600
[    0.549563] [drm:intel_dp_init_panel_power_sequencer.part.19] backlight on delay 1, off delay 200
[    0.549569] [drm:intel_dp_init_panel_power_sequencer_registers] panel power sequencer register settings: PP_ON 0x7d00001, PP_OFF 0x1f40001, PP_DIV 0x60
[    0.549577] [drm:edp_panel_vdd_on] Turning eDP port A VDD on
[    0.549580] [drm:wait_panel_power_cycle] Wait for panel power cycle
[    0.601112] [drm:wait_panel_status] mask b800000f value 00000000 status 00000000 control 00000062
[    0.601116] [drm:wait_panel_status] Wait complete
[    0.601121] [drm:edp_panel_vdd_on] PP_STATUS: 0x00000000 PP_CONTROL: 0x0000006a
[    0.601125] [drm:edp_panel_vdd_on] eDP port A panel power wasn't enabled
[    0.636720] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.645328] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.653926] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.662531] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.671132] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.679767] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.688366] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.696969] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.705572] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.714173] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.722770] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.731378] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.739983] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.748590] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.757210] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.765808] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.774422] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.783039] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.791642] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.800252] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.808859] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.817481] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.826086] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.834722] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.843335] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.851944] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.860552] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.869160] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.877767] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.886429] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.895045] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.903658] [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x7d4003ff
[    0.903665] [drm:drm_dp_dpcd_access] Too many retries, giving up. First error: -110
[    0.903666] [drm] failed to retrieve link info, disabling eDP
[    0.903671] [drm:edp_panel_vdd_off_sync] Turning eDP port A VDD off
[    0.903676] [drm:edp_panel_vdd_off_sync] PP_STATUS: 0x00000000 PP_CONTROL: 0x00000062
[    0.903695] [drm:intel_hdmi_init_connector] Adding HDMI connector on port B
[    0.903702] [drm:intel_hdmi_init_connector] Using DDC pin 0x1 for port B (VBT)
[    0.903720] [drm:intel_hdmi_init_connector] Adding HDMI connector on port C
[    0.903724] [drm:intel_hdmi_init_connector] Using DDC pin 0x2 for port C (VBT)
[    0.903728] [drm:intel_dsi_init] 
[    0.903733] [drm:acrngt_dom0_ready] gvt: core: acrngt: Dom 0 ready to accept Dom U guests

Thanks,
Feng
Feng Tang May 9, 2018, 6:31 a.m. UTC | #11
On Tue, May 08, 2018 at 10:30:19PM +0300, Jani Nikula wrote:
> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
> >  >> > > > 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.
> >
> > Hi Daniel,
> >
> > Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
> > based NUC. but I don't have the knowledge to tell if the VBT is broken :)
> 
> Please run current upstream drm-tip when you're suggesting changes to
> upstream code. Looks like you're running at most v4.14. This can't be
> emphasized enough. We can't and won't merge the changes unless they make
> sense with current code.

Yes, I understand, the patch posted  was created right after git-pulling
Linus' tree, and back-ported to test with 4.14 kernel. 

> 
> As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt.

Sure. as attached

Thanks,
Feng
Jani Nikula May 9, 2018, 7:53 a.m. UTC | #12
On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
> On Tue, May 08, 2018 at 10:30:19PM +0300, Jani Nikula wrote:
>> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
>> >> 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.
>> >
>> > Hi Daniel,
>> >
>> > Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
>> > based NUC. but I don't have the knowledge to tell if the VBT is broken :)
>> 
>> Please run current upstream drm-tip when you're suggesting changes to
>> upstream code. Looks like you're running at most v4.14. This can't be
>> emphasized enough. We can't and won't merge the changes unless they make
>> sense with current code.
>
> Yes, I understand, the patch posted  was created right after git-pulling
> Linus' tree, and back-ported to test with 4.14 kernel. 
>
>> 
>> As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt.
>
> Sure. as attached

Your VBT claims the device has an eDP panel. Does it have one or not?

BR,
Jani.
Feng Tang May 9, 2018, 7:56 a.m. UTC | #13
On Wed, May 09, 2018 at 10:53:53AM +0300, Jani Nikula wrote:
> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
> > On Tue, May 08, 2018 at 10:30:19PM +0300, Jani Nikula wrote:
> >> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
> >> >> 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.
> >> >
> >> > Hi Daniel,
> >> >
> >> > Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
> >> > based NUC. but I don't have the knowledge to tell if the VBT is broken :)
> >> 
> >> Please run current upstream drm-tip when you're suggesting changes to
> >> upstream code. Looks like you're running at most v4.14. This can't be
> >> emphasized enough. We can't and won't merge the changes unless they make
> >> sense with current code.
> >
> > Yes, I understand, the patch posted  was created right after git-pulling
> > Linus' tree, and back-ported to test with 4.14 kernel. 
> >
> >> 
> >> As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt.
> >
> > Sure. as attached
> 
> Your VBT claims the device has an eDP panel. Does it have one or not?

After asking around, our platform (BXT NUC) does have a eDP interface (someone
has tested with a eDP screen), but most of our platforms are connected
with 2 HDMI LCD monitors.

Thanks,
Feng
Jani Nikula May 9, 2018, 9:28 a.m. UTC | #14
On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
> On Wed, May 09, 2018 at 10:53:53AM +0300, Jani Nikula wrote:
>> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
>> > On Tue, May 08, 2018 at 10:30:19PM +0300, Jani Nikula wrote:
>> >> On Wed, 09 May 2018, Feng Tang <feng.tang@intel.com> wrote:
>> >> >> 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.
>> >> >
>> >> > Hi Daniel,
>> >> >
>> >> > Here are some of the VBT and DPCD related logs on my A3900 (bxt + GEN9 LP)
>> >> > based NUC. but I don't have the knowledge to tell if the VBT is broken :)
>> >> 
>> >> Please run current upstream drm-tip when you're suggesting changes to
>> >> upstream code. Looks like you're running at most v4.14. This can't be
>> >> emphasized enough. We can't and won't merge the changes unless they make
>> >> sense with current code.
>> >
>> > Yes, I understand, the patch posted  was created right after git-pulling
>> > Linus' tree, and back-ported to test with 4.14 kernel. 
>> >
>> >> 
>> >> As to vbt, please send me /sys/kernel/debug/dri/0/i915_vbt.
>> >
>> > Sure. as attached
>> 
>> Your VBT claims the device has an eDP panel. Does it have one or not?
>
> After asking around, our platform (BXT NUC) does have a eDP interface (someone
> has tested with a eDP screen), but most of our platforms are connected
> with 2 HDMI LCD monitors.

Sounds like you should have a different VBT for the cases where you ship
with/without eDP connected. As you've noticed, we generally try pretty
hard to talk to the panel if VBT says it's there, to avoid black screens
which are a much worse scenario than delays in detection.

BR,
Jani.

>
> Thanks,
> Feng
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffe14ec..6054067 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -171,6 +171,20 @@  EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
 #define AUX_RETRY_INTERVAL 500 /* us */
 
+/*
+ * The specification doesn't give any recommendation on how often to
+ * retry native transactions. We used to retry 7 times like for
+ * aux i2c transactions but real world devices this wasn't
+ * sufficient, bump to 32 which makes Dell 4k monitors happier.
+ *
+ * Since the retry may take quite some boot time, we make it a
+ * adjustable parameter for possible shorter retry time.
+ */
+static int dp_dpcd_max_retries __read_mostly = 32;
+module_param_unsafe(dp_dpcd_max_retries, int, 0644);
+MODULE_PARM_DESC(dp_dpcd_max_retries,
+		 "Max retry number for DPCD access (default 32)");
+
 /**
  * DOC: dp helpers
  *
@@ -198,13 +212,7 @@  static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 
 	mutex_lock(&aux->hw_mutex);
 
-	/*
-	 * The specification doesn't give any recommendation on how often to
-	 * retry native transactions. We used to retry 7 times like for
-	 * aux i2c transactions but real world devices this wasn't
-	 * sufficient, bump to 32 which makes Dell 4k monitors happier.
-	 */
-	for (retry = 0; retry < 32; retry++) {
+	for (retry = 0; retry < dp_dpcd_max_retries; retry++) {
 		if (ret != 0 && ret != -ETIMEDOUT) {
 			usleep_range(AUX_RETRY_INTERVAL,
 				     AUX_RETRY_INTERVAL + 100);