On Fri, Dec 11, 2015 at 07:10:35AM +0000, Wang, Gary C wrote:
> I will upload new version of patch for review. Thanks!
>
> -----Original Message-----
> From: Intel-gfx [mailto:[email protected]] On Behalf Of
> Wang, Gary C
> Sent: Friday, December 11, 2015 2:23 PM
> To: Jindal, Sonika <[email protected]>; [email protected]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correct max delay for HDMI hotplug
> live status checking
>
> No problem.
>
> Please refer following information.
> Lenovo Mode: L246 1xwA (4/30 failed, necessary hot-plug delay: 58/40/60/40ms)
> Philips Model : HH2AP (9/30 failed, necessary hot-plug delay:
> 80/50/50/60/46/40/58/58/39ms) BENQ Model:ET-0035-N (6/30 failed, necessary
> hot-plug delay: 60/50/50/80/80/40ms) DELL Model:U2713HM (2/30 failed,
> necessary hot-plug delay: 58/59ms) HP Model: HP-LP2475w (5/30 failed,
> necessary hot-plug delay: 70/50/40/60/40ms)
>
> Those test were based on following test-case (test continually with 30 test
> cycles), 1. HDMI display correctly 2. Un-plug/re-plug HDMI cable 3. Wait for
> one sec
>
> Thanks for your review!
>
> Gary
>
> -----Original Message-----
> From: Jindal, Sonika
> Sent: Friday, December 11, 2015 2:18 PM
> To: Wang, Gary C <[email protected]>; [email protected]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915: Correct max delay for HDMI hotplug
> live status checking
>
> Sure.
> Can you please also post your findings about different panels taking
> different time to settle the live status so that we know that this increase
> in retrials is helping you with multiple monitors?
>
> Regards,
> Sonika
>
>
> -----Original Message-----
> From: Wang, Gary C
> Sent: Friday, December 11, 2015 11:34 AM
> To: Jindal, Sonika <[email protected]>; [email protected]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915: Correct max delay for HDMI hotplug
> live status checking
>
> Hi Sonika,
>
> Thanks for your comment, and it's more readability on code.
>
> But it only delay 2 integration of each 10ms among checking hot-plug with 3
> times. Could I change it to following snippet code to read hotplug_status 4
> times at most for 3 integration 10ms delay?
>
> unsigned int retry = 3;
>
> do {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> if (live_status || !retry)
> break;
> mdelay(10);
> } while (retry--);
>
> Thanks!
>
> Gary
>
> -----Original Message-----
> From: Jindal, Sonika
> Sent: Friday, December 11, 2015 1:05 PM
> To: Wang, Gary C <[email protected]>; [email protected]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915: Correct max delay for HDMI hotplug
> live status checking
>
> How about following instead of two levels of check in the while loop:
>
> unsigned int retry = 3;
>
> do {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> if (live_status)
> break;
> mdelay(10);
> } while (--retry);
>
> Regards,
> Sonika
>
> -----Original Message-----
> From: Intel-gfx [mailto:[email protected]] On Behalf Of
> Wang, Gary C
> Sent: Friday, December 11, 2015 7:39 AM
> To: [email protected]
> Subject: [Intel-gfx] [PATCH] drm/i915: Correct max delay for HDMI hotplug
> live status checking
>
> The total delay of HDMI hotplug detecting with 30ms should have been split
> into a resolution of 3 retries of 10ms each, for the worst cases. But it
> still suffered from only waiting 10ms at most in intel_hdmi_detect(). This
> patch corrects it by reading hotplug status with 4 times at most for 30ms
> delay.
>
> Reviewed-by: Cooper Chiou <[email protected]>
> Cc: Gavin Hindman <[email protected]>
> Signed-off-by: Gary Wang <[email protected]>
Can we please have less top-posting when reviewing patches? I can't really
read the above with my bare-knuckles console mail client I use for reading
patch mailing lists ;-)
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-) mode change 100644 =>
> 100755 drivers/gpu/drm/i915/intel_hdmi.c
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> old mode 100644
> new mode 100755
> index be7fab9..888401b
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1387,16 +1387,19 @@ intel_hdmi_detect(struct drm_connector *connector,
> bool force)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> bool live_status = false;
> - unsigned int retry = 3;
> + // read hotplug status 4 times at most for 30ms delay (3 retries of
> 10ms each)
Please no C++ comments, also your code should be self-explanatory enough
that this is obvious.
> + unsigned int retry = 4;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - while (!live_status && --retry) {
> + while (!live_status && retry--) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> + if (live_status || !retry)
> + break;
> mdelay(10);
Whiel at it please review my patch to switch this one to an msleep - it's
not nice to kill the battery with a busy loop like this.
-Daniel
> }
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
